[PATCH] Fastcgi: core dump was caused by duplicated request header

[Simon Liu]

Thanks for your review.

this is new patch:

Index: src/http/modules/ngx_http_fastcgi_module.c
===================================================================
--- src/http/modules/ngx_http_fastcgi_module.c    (revision 3937)
+++ src/http/modules/ngx_http_fastcgi_module.c    (working copy)
@@ -165,7 +165,10 @@
 static char *ngx_http_fastcgi_lowat_check(ngx_conf_t *cf, void *post,
     void *data);

+static ngx_inline ngx_int_t ngx_http_fastcgi_ignored_header(ngx_table_elt_t
**ignored,
+    ngx_table_elt_t *header, ngx_uint_t header_params);

+
 static ngx_conf_post_t  ngx_http_fastcgi_lowat_post =
     { ngx_http_fastcgi_lowat_check };

@@ -684,6 +687,26 @@
 #endif


+static ngx_inline ngx_int_t
+ngx_http_fastcgi_ignored_header(ngx_table_elt_t **ignored, ngx_table_elt_t
*header, ngx_uint_t header_params)
+{
+    ngx_uint_t          n;
+    ngx_table_elt_t    *h;
+
+    for (n = 0; n < header_params; n++) {
+        h = ignored[n];
+
+        if (header->key.len == h->key.len
+            && ngx_memcmp(header->lowcase_key, h->lowcase_key,
header->key.len) == 0) {
+
+            return NGX_OK;
+        }
+    }
+
+    return NGX_DECLINED;
+}
+
+
 static ngx_int_t
 ngx_http_fastcgi_create_request(ngx_http_request_t *r)
 {
@@ -784,6 +807,11 @@
                 }

                 if (ngx_hash_find(&flcf->headers_hash, hash, lowcase_key,
n)) {
+
+                    if (ngx_http_fastcgi_ignored_header(ignored,
&header[i], header_params) == NGX_OK) {
+                        continue;
+                    }
+
                     ignored[header_params++] = &header[i];
                     continue;
                 }
@@ -915,10 +943,8 @@
                 i = 0;
             }

-            for (n = 0; n < header_params; n++) {
-                if (&header[i] == ignored[n]) {
-                    goto next;
-                }
+            if (ngx_http_fastcgi_ignored_header(ignored, &header[i],
header_params) == NGX_OK) {
+                continue;
             }

             key_len = sizeof("HTTP_") - 1 + header[i].key.len;
@@ -964,9 +990,6 @@
                            "fastcgi param: \"%*s: %*s\"",
                            key_len, b->last - (key_len + val_len),
                            val_len, b->last - val_len);
-        next:
-
-            continue;
         }
     }



On Wed, Jun 1, 2011 at 7:56 AM, Maxim Dounin <mdounin at mdounin.ru> wrote:

> Hello!
>
> On Wed, Jun 01, 2011 at 12:24:10AM +0800, Simon Liu wrote:
>
> > this bug will give rise to nginx(version >= 0.8.40) core dump, and it was
> > caused by this feature:
> >
> >     *) Feature: a "fastcgi_param" directive with value starting with
> >        "HTTP_" overrides a client request header line.
> >
> >
> > When we difine fastcgi_param directive with value starting with "HTTP_",
> > nginx  malloc a array(size is header_params that is number of value
> starting
> > with "HTTP_"), and if request header contain this value(HTTTP_xxx), nginx
> > will add this header pointer to array, but if header is duplicated, this
> > array will cross-border.
> >
> > e.g. if the config contain this directive (fastcgi_param HTTP_HOST
> > $http_host), and then request header send multi-duplicated header(Host),
> > nginx will core dump.
>
> Yes, thank you, it's known problem.
>
> [...]
>
> > +static ngx_inline ngx_int_t
> > +ngx_http_fastcgi_ignored_header(ngx_uint_t hash, ngx_uint_t
> header_params,
> > ngx_uint_t *ignored)
> > +{
> > +    ngx_uint_t          n;
> > +
> > +    for (n = 0; n < header_params; n++) {
> > +        if (hash == ignored[n]) {
> > +            return NGX_OK;
>
> You can't rely on hash here, as it's expected to have collisions.
>
> [...]
>
> > @@ -2374,7 +2363,22 @@
> >
> >              hk->key.len = src[i].key.len - 5;
> >              hk->key.data = src[i].key.data + 5;
> > -            hk->key_hash = ngx_hash_key_lc(hk->key.data, hk->key.len);
> > +
> > +            params_hash = 0;
> > +            for (n = 5; n < src[i].key.len; n++) {
> > +                ch = src[i].key.data[n];
> > +
> > +                if (ch >= 'A' && ch <= 'Z') {
> > +                    ch |= 0x20;
> > +
> > +                } else if (ch == '_') {
> > +                    ch = '-';
> > +                }
>
> This makes impossible to overwrite headers with real underscores
> (if underscores_in_headers are allowed).
>
> Maxim Dounin
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://nginx.org/mailman/listinfo/nginx-devel
>



-- 
博观约取

豆瓣:www.douban.com/people/mustang/

blog: www.pagefault.info

twitter: www.twitter.com/minibobo

sina 微博:  www.weibo.com/diaoliang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://nginx.org/pipermail/nginx-devel/attachments/20110602/8087264a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fastcgi.patch
Type: text/x-patch
Size: 2279 bytes
Desc: not available
URL: <http://nginx.org/pipermail/nginx-devel/attachments/20110602/8087264a/attachment.bin>