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

Simon Liu simohayha.bobo at gmail.com
Fri Jun 10 12:55:55 MSD 2011


Thanks!

I refactor the prev patch.

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,6 +165,8 @@
 static char *ngx_http_fastcgi_lowat_check(ngx_conf_t *cf, void *post,
     void *data);

+static ngx_int_t ngx_http_fastcgi_ignored_header(ngx_table_elt_t **ignored,
+    ngx_table_elt_t *header, ngx_uint_t params, ngx_uint_t
allow_underscores);

 static ngx_conf_post_t  ngx_http_fastcgi_lowat_post =
     { ngx_http_fastcgi_lowat_check };
@@ -685,13 +687,72 @@


 static ngx_int_t
+ngx_http_fastcgi_ignored_header(ngx_table_elt_t **ignored,
+    ngx_table_elt_t *header, ngx_uint_t params, ngx_uint_t
allow_underscores)
+{
+    u_char              k1, k2;
+    ngx_uint_t          n, i, duplicate;
+    ngx_table_elt_t    *h;
+
+    for (n = 0; n < params; n++) {
+        h = ignored[n];
+
+        if (h == header) {
+            return NGX_OK;
+        }
+
+        if (header->key.len != h->key.len) {
+            continue;
+        }
+
+        if (allow_underscores) {
+            duplicate = 1;
+
+            for (i = 0; i < header->key.len; i++) {
+
+                k1 = header->lowcase_key[i];
+                k2 = h->lowcase_key[i];
+
+                if (k1 == k2) {
+                    continue;
+                }
+
+                if ((k1 == '_' && k2 == '-') || (k1 == '-' && k2 == '_')) {
+                    continue;
+                }
+
+                duplicate = 0;
+                break;
+            }
+
+            if (duplicate) {
+                return NGX_OK;
+            }
+
+        } else {
+
+            if (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)
 {
     off_t                         file_pos;
     u_char                        ch, *pos, *lowcase_key;
     size_t                        size, len, key_len, val_len, padding,
                                   allocated;
-    ngx_uint_t                    i, n, next, hash, header_params;
+    ngx_uint_t                    i, n, next, hash, header_params,
+                                  allow_underscores;
     ngx_buf_t                    *b;
     ngx_chain_t                  *cl, *body;
     ngx_list_part_t              *part;
@@ -699,6 +760,7 @@
     ngx_http_script_code_pt       code;
     ngx_http_script_engine_t      e, le;
     ngx_http_fastcgi_header_t    *h;
+    ngx_http_core_srv_conf_t     *cscf;
     ngx_http_fastcgi_loc_conf_t  *flcf;
     ngx_http_script_len_code_pt   lcode;

@@ -707,7 +769,10 @@
     ignored = NULL;

     flcf = ngx_http_get_module_loc_conf(r, ngx_http_fastcgi_module);
+    cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);

+    allow_underscores = cscf->underscores_in_headers;
+
     if (flcf->params_len) {
         ngx_memzero(&le, sizeof(ngx_http_script_engine_t));

@@ -784,6 +849,14 @@
                 }

                 if (ngx_hash_find(&flcf->headers_hash, hash, lowcase_key,
n)) {
+
+                    if (header_params == flcf->header_params
+                       || ngx_http_fastcgi_ignored_header(ignored,
&header[i],
+                           header_params, allow_underscores) == NGX_OK)
+                    {
+                        continue;
+                    }
+
                     ignored[header_params++] = &header[i];
                     continue;
                 }
@@ -915,10 +988,10 @@
                 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, allow_underscores) == NGX_OK)
+            {
+                continue;
             }

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


On Fri, Jun 10, 2011 at 2:53 PM, Simon Liu <simohayha.bobo at gmail.com> wrote:

> Thanks!
>
>
> 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_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
> allow_underscores)
>
>
> +
>  static ngx_conf_post_t  ngx_http_fastcgi_lowat_post =
>      { ngx_http_fastcgi_lowat_check };
>
> @@ -685,6 +688,57 @@
>
>
>  static 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 allow_underscores)
> +{
> +    ngx_uint_t          n, i, duplicate;
> +    ngx_table_elt_t    *h;
>
> +
> +    for (n = 0; n < header_params; n++) {
> +        h = ignored[n];
> +
> +        if (h == header) {
> +            return NGX_OK;
> +        }
> +
> +        if (header->key.len != h->key.len) {
> +            continue;
> +        }
> +
> +        if (allow_underscores) {
> +            duplicate = 1;
> +
> +            for (i = 0; i < header->key.len; i++) {
> +
> +                if (header->lowcase_key[i] != h->lowcase_key[i]) {
> +                    if ((header->lowcase_key[i] == '_' &&
> h->lowcase_key[i] == '-')
> +                         || (header->lowcase_key[i] == '-' &&
> h->lowcase_key[i] == '_')) {
> +                        continue;
> +                    }
> +
> +                    duplicate = 0;
> +                    break;
> +                }
> +            }
> +
> +            if (duplicate) {
> +                return NGX_OK;
> +            }
> +
> +        } else {
> +
> +            if (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)
>  {
>      off_t                         file_pos;
> @@ -699,6 +753,7 @@
>
>      ngx_http_script_code_pt       code;
>      ngx_http_script_engine_t      e, le;
>      ngx_http_fastcgi_header_t    *h;
> +    ngx_http_core_srv_conf_t     *cscf;
>      ngx_http_fastcgi_loc_conf_t  *flcf;
>      ngx_http_script_len_code_pt   lcode;
>
> @@ -707,6 +762,7 @@
>      ignored = NULL;
>
>      flcf = ngx_http_get_module_loc_conf(r, ngx_http_fastcgi_module);
> +    cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
>
>      if (flcf->params_len) {
>          ngx_memzero(&le, sizeof(ngx_http_script_engine_t));
> @@ -784,6 +840,13 @@
>
>                  }
>
>                  if (ngx_hash_find(&flcf->headers_hash, hash, lowcase_key,
> n)) {
> +
> +                    if (header_params == flcf->header_params ||
> +                        ngx_http_fastcgi_ignored_header(ignored,
> &header[i],
> +                            header_params, cscf->underscores_in_headers)
> == NGX_OK) {
>
> +                        continue;
> +                    }
> +
>                      ignored[header_params++] = &header[i];
>                      continue;
>                  }
> @@ -915,10 +978,9 @@
>
>                  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, cscf->underscores_in_headers) ==
> NGX_OK) {
>
> +                continue;
>              }
>
>              key_len = sizeof("HTTP_") - 1 + header[i].key.len;
> @@ -964,9 +1026,6 @@
>
>                             "fastcgi param: \"%*s: %*s\"",
>                             key_len, b->last - (key_len + val_len),
>                             val_len, b->last - val_len);
> -        next:
> -
> -            continue;
>          }
>      }
>
>
>
> On Thu, Jun 2, 2011 at 5:52 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
>
>> Hello!
>>
>> On Thu, Jun 02, 2011 at 03:28:50PM +0800, Simon Liu wrote:
>>
>> > Thanks for your review.
>> >
>> > this is new patch:
>>
>> [...]
>>
>> > +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;
>>
>> This relies on lowcase_key of the first added header and the
>> duplicate one to match, but it's may not be true, e.g.
>>
>> X-Blah-Blah
>> X_Blah_Blah
>>
>> would have non-matching lowcase_key (but both should be ignored,
>> as they both maps to HTTP_BLAH_BLAH fastcgi key).  Request with
>> such duplicate headers will cause the same buffer overflow as in
>> the original bug (again, assuming underscores_in_headers is on).
>>
>> 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
>
>


-- 
博观约取

豆瓣: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/20110610/4037bd9b/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fastcgi.patch
Type: text/x-patch
Size: 4423 bytes
Desc: not available
URL: <http://nginx.org/pipermail/nginx-devel/attachments/20110610/4037bd9b/attachment-0001.bin>


More information about the nginx-devel mailing list