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

Simon Liu simohayha.bobo at gmail.com
Tue May 31 20:24:10 MSD 2011


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.


Index: src/http/modules/ngx_http_fastcgi_module.c
===================================================================
--- src/http/modules/ngx_http_fastcgi_module.c    (revision 3929)
+++ 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_uint_t
hash,
+    ngx_uint_t header_params, ngx_uint_t *ignored);

+
 static ngx_conf_post_t  ngx_http_fastcgi_lowat_post =
     { ngx_http_fastcgi_lowat_check };

@@ -684,18 +687,32 @@
 #endif


+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;
+        }
+    }
+
+    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;
+    u_char                        ch, *pos;
+    size_t                        size, len, key_len, val_len, padding;
+    ngx_uint_t                    i, n, next, header_params, *ignored;
     ngx_buf_t                    *b;
     ngx_chain_t                  *cl, *body;
     ngx_list_part_t              *part;
-    ngx_table_elt_t              *header, **ignored;
+    ngx_table_elt_t              *header;
     ngx_http_script_code_pt       code;
     ngx_http_script_engine_t      e, le;
     ngx_http_fastcgi_header_t    *h;
@@ -733,11 +750,8 @@

     if (flcf->upstream.pass_request_headers) {

-        allocated = 0;
-        lowcase_key = NULL;
-
         if (flcf->header_params) {
-            ignored = ngx_palloc(r->pool, flcf->header_params * sizeof(void
*));
+            ignored = ngx_palloc(r->pool, flcf->header_params *
sizeof(ngx_uint_t));
             if (ignored == NULL) {
                 return NGX_ERROR;
             }
@@ -759,43 +773,21 @@
             }

             if (flcf->header_params) {
-                if (allocated < header[i].key.len) {
-                    allocated = header[i].key.len + 16;
-                    lowcase_key = ngx_pnalloc(r->pool, allocated);
-                    if (lowcase_key == NULL) {
-                        return NGX_ERROR;
-                    }
-                }

-                hash = 0;
-
-                for (n = 0; n < header[i].key.len; n++) {
-                    ch = header[i].key.data[n];
-
-                    if (ch >= 'A' && ch <= 'Z') {
-                        ch |= 0x20;
-
-                    } else if (ch == '-') {
-                        ch = '_';
+                if (ngx_hash_find(&flcf->headers_hash, header[i].hash,
+                                  header[i].lowcase_key,
header[i].key.len)) {
+                    if (ngx_http_fastcgi_ignored_header(header[i].hash,
+                            header_params, ignored) == NGX_OK) {
+                        continue;
                     }
-
-                    hash = ngx_hash(hash, ch);
-                    lowcase_key[n] = ch;
-                }
-
-                if (ngx_hash_find(&flcf->headers_hash, hash, lowcase_key,
n)) {
-                    ignored[header_params++] = &header[i];
+                    ignored[header_params++] = header[i].hash;
                     continue;
                 }
-
-                n += sizeof("HTTP_") - 1;
-
-            } else {
-                n = sizeof("HTTP_") - 1 + header[i].key.len;
             }
-
-            len += ((n > 127) ? 4 : 1) + ((header[i].value.len > 127) ? 4 :
1)
-                + n + header[i].value.len;
+
+            len += ((sizeof("HTTP_") - 1 + header[i].key.len > 127) ? 4 :
1)
+                + ((header[i].value.len > 127) ? 4 : 1)
+                + sizeof("HTTP_") - 1 + header[i].key.len +
header[i].value.len;
         }
     }

@@ -915,10 +907,9 @@
                 i = 0;
             }

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

             key_len = sizeof("HTTP_") - 1 + header[i].key.len;
@@ -964,8 +955,6 @@
                            "fastcgi param: \"%*s: %*s\"",
                            key_len, b->last - (key_len + val_len),
                            val_len, b->last - val_len);
-        next:
-
             continue;
         }
     }
@@ -2013,10 +2002,10 @@
     ngx_http_fastcgi_loc_conf_t *prev = parent;
     ngx_http_fastcgi_loc_conf_t *conf = child;

-    u_char                       *p;
+    u_char                        ch, *p;
     size_t                        size;
     uintptr_t                    *code;
-    ngx_uint_t                    i;
+    ngx_uint_t                    i, n, params_hash;
     ngx_array_t                   headers_names;
     ngx_keyval_t                 *src;
     ngx_hash_key_t               *hk;
@@ -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 = '-';
+                }
+
+                params_hash = ngx_hash(params_hash, ch);
+            }
+
+            hk->key_hash = params_hash;
             hk->value = (void *) 1;

             if (src[i].value.len == 0) {

-- 
博观约取

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


More information about the nginx-devel mailing list