[PATCH] Fix a memory invalid read issue in ngx_http_gzip_ok

[Maxim Dounin]

Hello!

On Thu, Dec 22, 2011 at 11:19:00AM +0800, agentzh wrote:

> Hello!
> 
> Here attaches a patch for ngx_http_core_module (of the Nginx 1.0.10
> core) to fix a memory invalid read bug captured by the valgrind
> memcheck tool on my side.
> 
> When the Accept-Encoding request header takes the exact "gzip" value,
> the ngx_http_gzip_ok function might run out of the memory block by 1
> byte when calling ngx_memcmp to compare exactly 5 bytes of data.
> 
> Hopefully this patch can be applied to the mainstream nginx :)
> 
> Thanks!
> -agentzh
> 
> --- nginx-1.0.10/src/http/ngx_http_core_module.c	2011-11-01
> 21:45:33.000000000 +0800
> +++ nginx-1.0.10-patched/src/http/ngx_http_core_module.c	2011-12-22
> 11:08:02.546297974 +0800
> @@ -2070,7 +2070,7 @@
>       *   Opera:   "gzip, deflate"
>       */
> 
> -    if (ngx_memcmp(ae->value.data, "gzip,", 5) != 0
> +    if (ngx_memcmp(ae->value.data, "gzip,", ngx_min(ae->value.len, 5)) != 0
>          && ngx_http_gzip_accept_encoding(&ae->value) != NGX_OK)
>      {
>          return NGX_DECLINED;

There is no problem here.  The "ae" is from r->headers_in and must 
be null-terminated.  And ae->value.len is checked in previous 
condition to be at least 4 bytes, i.e. at least 5 bytes can be 
read ae->value.data.

I suppose the problem is instead in your code which (incorrectly) 
adds headers to r->headers_in.

(And the patch is wrong even if one assume we need extra checks 
here: it should check value.len instead of capping one passed to 
ngx_memcmp.)

Maxim Dounin