procol version integer overflow, downgrade to 0.9

[openid.stackexchange.com/user/9a01f091-0d6d-4e99-8f37-dcf99897dd7c]

Nginx currently supports the old RFC with :

HTTP / *DIGIT . *DIGIT

But when extracting the major and minor version there's an int16 overflow which means that currently "HTTP/65536.9" or "HTTP/65536.8" can be used and will be detected as HTTP/0.9.

This can be used to generate headless responses from Nginx (like a regular 0.9 query) while using something which does not look like a 0.9 query.

They're two ways of fixing it:

• use the attached patch to prevent int16 overflow
• remove the multi-digit part in the automaton parser (as the new rfc 7230 allows only one digit for major and 1 for minor)

Note that this patch is a poc on ngx_http_parse.c which may need to be applied on other places like ngx_event_openssl_stapling.c or  ngx_http_spdy.c where the same issue can also be present.

[Maxim Dounin]

The restriction in the suggested patch can be easily bypassed: the patch stores number of digits parsed on stack, while the ngx_http_parse_request_line() function can be called multiple times.

Slightly better patch below.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1463276652 -10800
#      Sun May 15 04:44:12 2016 +0300
# Node ID bf200866cde2099c1cbff31a5dcb32722ca62fb3
# Parent  f2491b0f35af80c91018c0930cbcfe5556453e35
Added overflow checks for version numbers (ticket #762).

Both minor and major versions are now limited to 999 maximum.  In case of
r->http_minor, this limit is already implied by the code.  Major version,
r->http_major, in theory can be up to 65535 with current code, but such
values are very unlikely to become real (and, additionally, such values
are not allowed by RFC 7230), so the same test was used for r->http_major.

diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c
--- a/src/http/ngx_http_parse.c
+++ b/src/http/ngx_http_parse.c
@@ -737,6 +737,10 @@ ngx_http_parse_request_line(ngx_http_req
                 return NGX_HTTP_PARSE_INVALID_REQUEST;
             }
 
+            if (r->http_major > 99) {
+                return NGX_HTTP_PARSE_INVALID_REQUEST;
+            }
+
             r->http_major = r->http_major * 10 + ch - '0';
             break;
 
@@ -770,6 +774,10 @@ ngx_http_parse_request_line(ngx_http_req
                 return NGX_HTTP_PARSE_INVALID_REQUEST;
             }
 
+            if (r->http_minor > 99) {
+                return NGX_HTTP_PARSE_INVALID_REQUEST;
+            }
+
             r->http_minor = r->http_minor * 10 + ch - '0';
             break;
 
@@ -1680,6 +1688,10 @@ ngx_http_parse_status_line(ngx_http_requ
                 return NGX_ERROR;
             }
 
+            if (r->http_major > 99) {
+                return NGX_ERROR;
+            }
+
             r->http_major = r->http_major * 10 + ch - '0';
             break;
 
@@ -1704,6 +1716,10 @@ ngx_http_parse_status_line(ngx_http_requ
                 return NGX_ERROR;
             }
 
+            if (r->http_minor > 99) {
+                return NGX_ERROR;
+            }
+
             r->http_minor = r->http_minor * 10 + ch - '0';
             break;
 

[Maxim Dounin <mdounin@…>]

In 6543:302ff40c9bc9/nginx:

Added overflow checks for version numbers (ticket #762).

Both minor and major versions are now limited to 999 maximum. In case of
r->http_minor, this limit is already implied by the code. Major version,
r->http_major, in theory can be up to 65535 with current code, but such
values are very unlikely to become real (and, additionally, such values
are not allowed by RFC 7230), so the same test was used for r->http_major.