Maxim Dounin
March 04, 2014 10:20AM
details: http://hg.nginx.org/nginx/rev/6808ea2d69e4
branches:
changeset: 5590:6808ea2d69e4
user: Valentin Bartenev <vbart@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;


_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

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

Maxim Dounin 509 March 04, 2014 10:20AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 140
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready