From 0c05e5b55fe799bb0d6330bcf75dbde370bf0ba6 Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Mon, 3 Mar 2014 19:24:55 +0400 Subject: [PATCH] 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. --- src/http/ngx_http_spdy.c | 58 +++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/src/http/ngx_http_spdy.c b/src/http/ngx_http_spdy.c index 2b7b89f18..4005bfbe7 100644 --- 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_request_t *r) return NGX_AGAIN; } - len = ngx_spdy_frame_parse_uint32(p); + r->lowcase_index = ngx_spdy_frame_parse_uint32(p); - if (!len) { + if (r->lowcase_index == 0) { return NGX_HTTP_PARSE_INVALID_HEADER; } @@ -2359,8 +2359,6 @@ ngx_http_spdy_parse_header(ngx_http_request_t *r) 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_request_t *r) 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_request_t *r) 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_request_t *r) 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_request_t *r) 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(ngx_http_request_t *r) 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_http_request_t *r) } 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_http_request_t *r) 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_request_t *r) 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_request_t *r) h->hash = r->header_hash; - h->key.len = r->lowcase_index; - h->key.data = &r->header_name_start[1]; + 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; @@ -2778,7 +2768,7 @@ ngx_http_spdy_parse_version(ngx_http_request_t *r) 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_request_t *r) 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;