[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