[nginx] SPDY: fixed potential integer overflow while parsing hea...

Maxim Dounin mdounin at mdounin.ru
Tue Mar 4 15:18:28 UTC 2014


details:   http://hg.nginx.org/nginx/rev/6808ea2d69e4
branches:  
changeset: 5590:6808ea2d69e4
user:      Valentin Bartenev <vbart at nginx.com>
date:      Mon Mar 03 19:24:55 2014 +0400
description:
SPDY: fixed potential integer overflow while parsing headers.

Previously r->header_size was used to store length for a part of
value that represents an individual already parsed HTTP header,
while r->header_end pointed to the end of the whole value.

Instead of storing length of a following name or value as pointer
to a potential end address (r->header_name_end and r->header_end)
that might be overflowed, now r->lowercase_index counter is used
to store remaining length of a following unparsed field.

It also fixes incorrect $body_bytes_sent value if a request is
closed while parsing of the request header.  Since r->header_size
is intended for counting header size, thus abusing it for header
parsing purpose was certainly a bad idea.

diffstat:

 src/http/ngx_http_spdy.c |  62 ++++++++++++++++++++---------------------------
 1 files changed, 26 insertions(+), 36 deletions(-)

diffs (196 lines):

diff --git a/src/http/ngx_http_spdy.c b/src/http/ngx_http_spdy.c
--- a/src/http/ngx_http_spdy.c
+++ b/src/http/ngx_http_spdy.c
@@ -2325,7 +2325,7 @@ static ngx_int_t
 ngx_http_spdy_parse_header(ngx_http_request_t *r)
 {
     u_char                     *p, *end, ch;
-    ngx_uint_t                  len, hash;
+    ngx_uint_t                  hash;
     ngx_http_core_srv_conf_t   *cscf;
 
     enum {
@@ -2348,9 +2348,9 @@ ngx_http_spdy_parse_header(ngx_http_requ
             return NGX_AGAIN;
         }
 
-        len = ngx_spdy_frame_parse_uint32(p);
-
-        if (!len) {
+        r->lowcase_index = ngx_spdy_frame_parse_uint32(p);
+
+        if (r->lowcase_index == 0) {
             return NGX_HTTP_PARSE_INVALID_HEADER;
         }
 
@@ -2359,8 +2359,6 @@ ngx_http_spdy_parse_header(ngx_http_requ
 
         p += NGX_SPDY_NV_NLEN_SIZE;
 
-        r->header_name_end = p + len;
-        r->lowcase_index = len;
         r->invalid_header = 0;
 
         state = sw_name;
@@ -2369,16 +2367,16 @@ ngx_http_spdy_parse_header(ngx_http_requ
 
     case sw_name:
 
-        if (r->header_name_end > end) {
+        if ((ngx_uint_t) (end - p) < r->lowcase_index) {
             break;
         }
 
         cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
 
         r->header_name_start = p;
+        r->header_name_end = p + r->lowcase_index;
 
         if (p[0] == ':') {
-            r->lowcase_index--;
             p++;
         }
 
@@ -2425,29 +2423,26 @@ ngx_http_spdy_parse_header(ngx_http_requ
             break;
         }
 
-        len = ngx_spdy_frame_parse_uint32(p);
+        r->lowcase_index = ngx_spdy_frame_parse_uint32(p);
 
         /* null-terminate header name */
         *p = '\0';
 
         p += NGX_SPDY_NV_VLEN_SIZE;
 
-        r->header_end = p + len;
-
         state = sw_value;
 
         /* fall through */
 
     case sw_value:
 
-        if (r->header_end > end) {
+        if ((ngx_uint_t) (end - p) < r->lowcase_index) {
             break;
         }
 
         r->header_start = p;
 
-        for ( /* void */ ; p != r->header_end; p++) {
-
+        while (r->lowcase_index--) {
             ch = *p;
 
             if (ch == '\0') {
@@ -2456,7 +2451,7 @@ ngx_http_spdy_parse_header(ngx_http_requ
                     return NGX_ERROR;
                 }
 
-                r->header_size = p - r->header_start;
+                r->header_end = p;
                 r->header_in->pos = p + 1;
 
                 return NGX_OK;
@@ -2465,9 +2460,11 @@ ngx_http_spdy_parse_header(ngx_http_requ
             if (ch == CR || ch == LF) {
                 return NGX_HTTP_PARSE_INVALID_HEADER;
             }
+
+            p++;
         }
 
-        r->header_size = p - r->header_start;
+        r->header_end = p;
         r->header_in->pos = p;
 
         r->state = 0;
@@ -2526,13 +2523,6 @@ ngx_http_spdy_alloc_large_header_buffer(
         buf->last = ngx_cpymem(new, old, rest);
     }
 
-    if (r->header_name_end > old) {
-        r->header_name_end = new + (r->header_name_end - old);
-
-    } else if (r->header_end > old) {
-        r->header_end = new + (r->header_end - old);
-    }
-
     r->header_in = buf;
 
     stream->header_buffers++;
@@ -2563,14 +2553,14 @@ ngx_http_spdy_handle_request_header(ngx_
     }
 
     if (r->header_name_start[0] == ':') {
+        r->header_name_start++;
+
         for (i = 0; i < NGX_SPDY_REQUEST_HEADERS; i++) {
             sh = &ngx_http_spdy_request_headers[i];
 
             if (sh->hash != r->header_hash
-                || sh->len != r->lowcase_index
-                || ngx_strncmp(sh->header, &r->header_name_start[1],
-                               r->lowcase_index)
-                   != 0)
+                || sh->len != r->header_name_end - r->header_name_start
+                || ngx_strncmp(sh->header, r->header_name_start, sh->len) != 0)
             {
                 continue;
             }
@@ -2590,10 +2580,10 @@ ngx_http_spdy_handle_request_header(ngx_
 
     h->hash = r->header_hash;
 
-    h->key.len = r->lowcase_index;
+    h->key.len = r->header_name_end - r->header_name_start;
     h->key.data = r->header_name_start;
 
-    h->value.len = r->header_size;
+    h->value.len = r->header_end - r->header_start;
     h->value.data = r->header_start;
 
     h->lowcase_key = h->key.data;
@@ -2653,7 +2643,7 @@ ngx_http_spdy_parse_method(ngx_http_requ
         return NGX_HTTP_PARSE_INVALID_HEADER;
     }
 
-    len = r->header_size;
+    len = r->header_end - r->header_start;
 
     r->method_name.len = len;
     r->method_name.data = r->header_start;
@@ -2733,10 +2723,10 @@ ngx_http_spdy_parse_host(ngx_http_reques
 
     h->hash = r->header_hash;
 
-    h->key.len = r->lowcase_index;
-    h->key.data = &r->header_name_start[1];
-
-    h->value.len = r->header_size;
+    h->key.len = r->header_name_end - r->header_name_start;
+    h->key.data = r->header_name_start;
+
+    h->value.len = r->header_end - r->header_start;
     h->value.data = r->header_start;
 
     h->lowcase_key = h->key.data;
@@ -2778,7 +2768,7 @@ ngx_http_spdy_parse_version(ngx_http_req
 
     p = r->header_start;
 
-    if (r->header_size < 8 || !(ngx_str5cmp(p, 'H', 'T', 'T', 'P', '/'))) {
+    if (r->header_end - p < 8 || !(ngx_str5cmp(p, 'H', 'T', 'T', 'P', '/'))) {
         return NGX_HTTP_PARSE_INVALID_REQUEST;
     }
 
@@ -2828,7 +2818,7 @@ ngx_http_spdy_parse_version(ngx_http_req
         r->http_minor = r->http_minor * 10 + ch - '0';
     }
 
-    r->http_protocol.len = r->header_size;
+    r->http_protocol.len = r->header_end - r->header_start;
     r->http_protocol.data = r->header_start;
     r->http_version = r->http_major * 1000 + r->http_minor;
 



More information about the nginx-devel mailing list