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

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel