From 83c063346d9ff7a4f449010c6f0460ef7171e5ef Mon Sep 17 00:00:00 2001 From: Michael Kaufmann Date: Tue, 31 Aug 2021 21:30:54 +0200 Subject: [PATCH] Stricter checks for pseudo-headers :method and :path Check the allowed characters for ":method" (see RFC 7230, section 3.2.6) and ":path". For ":path", the space and tab characters are now forbidden, but other special characters are still allowed for compatibility reasons. Update genvchartbl.py so that it generates the same table as in the code. Fixes #1611 --- genmethodchartbl.py | 29 ++++++ genpathchartbl.py | 23 +++++ genvchartbl.py | 2 - lib/includes/nghttp2/nghttp2.h | 25 ++++- lib/nghttp2_helper.c | 161 ++++++++++++++++++++++++++++++++- lib/nghttp2_http.c | 8 +- 6 files changed, 243 insertions(+), 5 deletions(-) create mode 100755 genmethodchartbl.py create mode 100755 genpathchartbl.py diff --git a/genmethodchartbl.py b/genmethodchartbl.py new file mode 100755 index 00000000..a0c3a378 --- /dev/null +++ b/genmethodchartbl.py @@ -0,0 +1,29 @@ +#!/usr/bin/env python3 +import sys + +def name(i): + if i < 0x21: + return \ + ['NUL ', 'SOH ', 'STX ', 'ETX ', 'EOT ', 'ENQ ', 'ACK ', 'BEL ', + 'BS ', 'HT ', 'LF ', 'VT ', 'FF ', 'CR ', 'SO ', 'SI ', + 'DLE ', 'DC1 ', 'DC2 ', 'DC3 ', 'DC4 ', 'NAK ', 'SYN ', 'ETB ', + 'CAN ', 'EM ', 'SUB ', 'ESC ', 'FS ', 'GS ', 'RS ', 'US ', + 'SPC '][i] + elif i == 0x7f: + return 'DEL ' + +for i in range(256): + if chr(i) in ["!" , "#" , "$" , "%" , "&" , "'" , "*", + "+" , "-" , "." , "^" , "_" , "`" , "|" , "~"] or\ + ('0' <= chr(i) and chr(i) <= '9') or \ + ('A' <= chr(i) and chr(i) <= 'Z') or \ + ('a' <= chr(i) and chr(i) <= 'z'): + sys.stdout.write('1 /* {} */, '.format(chr(i))) + elif (0x21 <= i and i < 0x7f): + sys.stdout.write('0 /* {} */, '.format(chr(i))) + elif 0x80 <= i: + sys.stdout.write('0 /* {} */, '.format(hex(i))) + else: + sys.stdout.write('0 /* {} */, '.format(name(i))) + if (i + 1)%4 == 0: + sys.stdout.write('\n') diff --git a/genpathchartbl.py b/genpathchartbl.py new file mode 100755 index 00000000..cd8c7144 --- /dev/null +++ b/genpathchartbl.py @@ -0,0 +1,23 @@ +#!/usr/bin/env python3 +import sys + +def name(i): + if i < 0x21: + return \ + ['NUL ', 'SOH ', 'STX ', 'ETX ', 'EOT ', 'ENQ ', 'ACK ', 'BEL ', + 'BS ', 'HT ', 'LF ', 'VT ', 'FF ', 'CR ', 'SO ', 'SI ', + 'DLE ', 'DC1 ', 'DC2 ', 'DC3 ', 'DC4 ', 'NAK ', 'SYN ', 'ETB ', + 'CAN ', 'EM ', 'SUB ', 'ESC ', 'FS ', 'GS ', 'RS ', 'US ', + 'SPC '][i] + elif i == 0x7f: + return 'DEL ' + +for i in range(256): + if (0x21 <= i and i < 0x7f): + sys.stdout.write('1 /* {} */, '.format(chr(i))) + elif 0x80 <= i: + sys.stdout.write('1 /* {} */, '.format(hex(i))) + else: + sys.stdout.write('0 /* {} */, '.format(name(i))) + if (i + 1)%4 == 0: + sys.stdout.write('\n') diff --git a/genvchartbl.py b/genvchartbl.py index 4e4d4d96..a4ff8d1f 100755 --- a/genvchartbl.py +++ b/genvchartbl.py @@ -20,8 +20,6 @@ for i in range(256): sys.stdout.write('1 /* {} */, '.format(chr(i))) elif 0x80 <= i: sys.stdout.write('1 /* {} */, '.format(hex(i))) - elif 0 == i: - sys.stdout.write('1 /* NUL */, ') else: sys.stdout.write('0 /* {} */, '.format(name(i))) if (i + 1)%4 == 0: diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index f93c5da5..a8b6c8b8 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -4839,7 +4839,30 @@ NGHTTP2_EXTERN int nghttp2_check_header_value(const uint8_t *value, size_t len); /** * @function * - * Returns nonzero if the |value| which is supposed to the value of + * Returns nonzero if the |value| which is supposed to be the value of the + * :method header field is valid according to + * https://datatracker.ietf.org/doc/html/rfc7231#section-4 and + * https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6 + */ +NGHTTP2_EXTERN int nghttp2_check_method(const uint8_t *value, size_t len); + +/** + * @function + * + * Returns nonzero if the |value| which is supposed to be the value of the + * :path header field is valid according to + * https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2.3 + * + * |value| is valid if it merely consists of the allowed characters. + * In particular, it does not check whether |value| follows the syntax + * of path. + */ +NGHTTP2_EXTERN int nghttp2_check_path(const uint8_t *value, size_t len); + +/** + * @function + * + * Returns nonzero if the |value| which is supposed to be the value of the * :authority or host header field is valid according to * https://tools.ietf.org/html/rfc3986#section-3.2 * diff --git a/lib/nghttp2_helper.c b/lib/nghttp2_helper.c index 0bd54147..588e269c 100644 --- a/lib/nghttp2_helper.c +++ b/lib/nghttp2_helper.c @@ -507,7 +507,166 @@ int nghttp2_check_header_value(const uint8_t *value, size_t len) { return 1; } -/* Generated by genauthroitychartbl.py */ +/* Generated by genmethodchartbl.py */ +static char VALID_METHOD_CHARS[] = { + 0 /* NUL */, 0 /* SOH */, 0 /* STX */, 0 /* ETX */, + 0 /* EOT */, 0 /* ENQ */, 0 /* ACK */, 0 /* BEL */, + 0 /* BS */, 0 /* HT */, 0 /* LF */, 0 /* VT */, + 0 /* FF */, 0 /* CR */, 0 /* SO */, 0 /* SI */, + 0 /* DLE */, 0 /* DC1 */, 0 /* DC2 */, 0 /* DC3 */, + 0 /* DC4 */, 0 /* NAK */, 0 /* SYN */, 0 /* ETB */, + 0 /* CAN */, 0 /* EM */, 0 /* SUB */, 0 /* ESC */, + 0 /* FS */, 0 /* GS */, 0 /* RS */, 0 /* US */, + 0 /* SPC */, 1 /* ! */, 0 /* " */, 1 /* # */, + 1 /* $ */, 1 /* % */, 1 /* & */, 1 /* ' */, + 0 /* ( */, 0 /* ) */, 1 /* * */, 1 /* + */, + 0 /* , */, 1 /* - */, 1 /* . */, 0 /* / */, + 1 /* 0 */, 1 /* 1 */, 1 /* 2 */, 1 /* 3 */, + 1 /* 4 */, 1 /* 5 */, 1 /* 6 */, 1 /* 7 */, + 1 /* 8 */, 1 /* 9 */, 0 /* : */, 0 /* ; */, + 0 /* < */, 0 /* = */, 0 /* > */, 0 /* ? */, + 0 /* @ */, 1 /* A */, 1 /* B */, 1 /* C */, + 1 /* D */, 1 /* E */, 1 /* F */, 1 /* G */, + 1 /* H */, 1 /* I */, 1 /* J */, 1 /* K */, + 1 /* L */, 1 /* M */, 1 /* N */, 1 /* O */, + 1 /* P */, 1 /* Q */, 1 /* R */, 1 /* S */, + 1 /* T */, 1 /* U */, 1 /* V */, 1 /* W */, + 1 /* X */, 1 /* Y */, 1 /* Z */, 0 /* [ */, + 0 /* \ */, 0 /* ] */, 1 /* ^ */, 1 /* _ */, + 1 /* ` */, 1 /* a */, 1 /* b */, 1 /* c */, + 1 /* d */, 1 /* e */, 1 /* f */, 1 /* g */, + 1 /* h */, 1 /* i */, 1 /* j */, 1 /* k */, + 1 /* l */, 1 /* m */, 1 /* n */, 1 /* o */, + 1 /* p */, 1 /* q */, 1 /* r */, 1 /* s */, + 1 /* t */, 1 /* u */, 1 /* v */, 1 /* w */, + 1 /* x */, 1 /* y */, 1 /* z */, 0 /* { */, + 1 /* | */, 0 /* } */, 1 /* ~ */, 0 /* DEL */, + 0 /* 0x80 */, 0 /* 0x81 */, 0 /* 0x82 */, 0 /* 0x83 */, + 0 /* 0x84 */, 0 /* 0x85 */, 0 /* 0x86 */, 0 /* 0x87 */, + 0 /* 0x88 */, 0 /* 0x89 */, 0 /* 0x8a */, 0 /* 0x8b */, + 0 /* 0x8c */, 0 /* 0x8d */, 0 /* 0x8e */, 0 /* 0x8f */, + 0 /* 0x90 */, 0 /* 0x91 */, 0 /* 0x92 */, 0 /* 0x93 */, + 0 /* 0x94 */, 0 /* 0x95 */, 0 /* 0x96 */, 0 /* 0x97 */, + 0 /* 0x98 */, 0 /* 0x99 */, 0 /* 0x9a */, 0 /* 0x9b */, + 0 /* 0x9c */, 0 /* 0x9d */, 0 /* 0x9e */, 0 /* 0x9f */, + 0 /* 0xa0 */, 0 /* 0xa1 */, 0 /* 0xa2 */, 0 /* 0xa3 */, + 0 /* 0xa4 */, 0 /* 0xa5 */, 0 /* 0xa6 */, 0 /* 0xa7 */, + 0 /* 0xa8 */, 0 /* 0xa9 */, 0 /* 0xaa */, 0 /* 0xab */, + 0 /* 0xac */, 0 /* 0xad */, 0 /* 0xae */, 0 /* 0xaf */, + 0 /* 0xb0 */, 0 /* 0xb1 */, 0 /* 0xb2 */, 0 /* 0xb3 */, + 0 /* 0xb4 */, 0 /* 0xb5 */, 0 /* 0xb6 */, 0 /* 0xb7 */, + 0 /* 0xb8 */, 0 /* 0xb9 */, 0 /* 0xba */, 0 /* 0xbb */, + 0 /* 0xbc */, 0 /* 0xbd */, 0 /* 0xbe */, 0 /* 0xbf */, + 0 /* 0xc0 */, 0 /* 0xc1 */, 0 /* 0xc2 */, 0 /* 0xc3 */, + 0 /* 0xc4 */, 0 /* 0xc5 */, 0 /* 0xc6 */, 0 /* 0xc7 */, + 0 /* 0xc8 */, 0 /* 0xc9 */, 0 /* 0xca */, 0 /* 0xcb */, + 0 /* 0xcc */, 0 /* 0xcd */, 0 /* 0xce */, 0 /* 0xcf */, + 0 /* 0xd0 */, 0 /* 0xd1 */, 0 /* 0xd2 */, 0 /* 0xd3 */, + 0 /* 0xd4 */, 0 /* 0xd5 */, 0 /* 0xd6 */, 0 /* 0xd7 */, + 0 /* 0xd8 */, 0 /* 0xd9 */, 0 /* 0xda */, 0 /* 0xdb */, + 0 /* 0xdc */, 0 /* 0xdd */, 0 /* 0xde */, 0 /* 0xdf */, + 0 /* 0xe0 */, 0 /* 0xe1 */, 0 /* 0xe2 */, 0 /* 0xe3 */, + 0 /* 0xe4 */, 0 /* 0xe5 */, 0 /* 0xe6 */, 0 /* 0xe7 */, + 0 /* 0xe8 */, 0 /* 0xe9 */, 0 /* 0xea */, 0 /* 0xeb */, + 0 /* 0xec */, 0 /* 0xed */, 0 /* 0xee */, 0 /* 0xef */, + 0 /* 0xf0 */, 0 /* 0xf1 */, 0 /* 0xf2 */, 0 /* 0xf3 */, + 0 /* 0xf4 */, 0 /* 0xf5 */, 0 /* 0xf6 */, 0 /* 0xf7 */, + 0 /* 0xf8 */, 0 /* 0xf9 */, 0 /* 0xfa */, 0 /* 0xfb */, + 0 /* 0xfc */, 0 /* 0xfd */, 0 /* 0xfe */, 0 /* 0xff */ +}; + +int nghttp2_check_method(const uint8_t *value, size_t len) { + const uint8_t *last; + if (len == 0) { + return 0; + } + for (last = value + len; value != last; ++value) { + if (!VALID_METHOD_CHARS[*value]) { + return 0; + } + } + return 1; +} + +/* Generated by genpathchartbl.py */ +static char VALID_PATH_CHARS[] = { + 0 /* NUL */, 0 /* SOH */, 0 /* STX */, 0 /* ETX */, + 0 /* EOT */, 0 /* ENQ */, 0 /* ACK */, 0 /* BEL */, + 0 /* BS */, 0 /* HT */, 0 /* LF */, 0 /* VT */, + 0 /* FF */, 0 /* CR */, 0 /* SO */, 0 /* SI */, + 0 /* DLE */, 0 /* DC1 */, 0 /* DC2 */, 0 /* DC3 */, + 0 /* DC4 */, 0 /* NAK */, 0 /* SYN */, 0 /* ETB */, + 0 /* CAN */, 0 /* EM */, 0 /* SUB */, 0 /* ESC */, + 0 /* FS */, 0 /* GS */, 0 /* RS */, 0 /* US */, + 0 /* SPC */, 1 /* ! */, 1 /* " */, 1 /* # */, + 1 /* $ */, 1 /* % */, 1 /* & */, 1 /* ' */, + 1 /* ( */, 1 /* ) */, 1 /* * */, 1 /* + */, + 1 /* , */, 1 /* - */, 1 /* . */, 1 /* / */, + 1 /* 0 */, 1 /* 1 */, 1 /* 2 */, 1 /* 3 */, + 1 /* 4 */, 1 /* 5 */, 1 /* 6 */, 1 /* 7 */, + 1 /* 8 */, 1 /* 9 */, 1 /* : */, 1 /* ; */, + 1 /* < */, 1 /* = */, 1 /* > */, 1 /* ? */, + 1 /* @ */, 1 /* A */, 1 /* B */, 1 /* C */, + 1 /* D */, 1 /* E */, 1 /* F */, 1 /* G */, + 1 /* H */, 1 /* I */, 1 /* J */, 1 /* K */, + 1 /* L */, 1 /* M */, 1 /* N */, 1 /* O */, + 1 /* P */, 1 /* Q */, 1 /* R */, 1 /* S */, + 1 /* T */, 1 /* U */, 1 /* V */, 1 /* W */, + 1 /* X */, 1 /* Y */, 1 /* Z */, 1 /* [ */, + 1 /* \ */, 1 /* ] */, 1 /* ^ */, 1 /* _ */, + 1 /* ` */, 1 /* a */, 1 /* b */, 1 /* c */, + 1 /* d */, 1 /* e */, 1 /* f */, 1 /* g */, + 1 /* h */, 1 /* i */, 1 /* j */, 1 /* k */, + 1 /* l */, 1 /* m */, 1 /* n */, 1 /* o */, + 1 /* p */, 1 /* q */, 1 /* r */, 1 /* s */, + 1 /* t */, 1 /* u */, 1 /* v */, 1 /* w */, + 1 /* x */, 1 /* y */, 1 /* z */, 1 /* { */, + 1 /* | */, 1 /* } */, 1 /* ~ */, 0 /* DEL */, + 1 /* 0x80 */, 1 /* 0x81 */, 1 /* 0x82 */, 1 /* 0x83 */, + 1 /* 0x84 */, 1 /* 0x85 */, 1 /* 0x86 */, 1 /* 0x87 */, + 1 /* 0x88 */, 1 /* 0x89 */, 1 /* 0x8a */, 1 /* 0x8b */, + 1 /* 0x8c */, 1 /* 0x8d */, 1 /* 0x8e */, 1 /* 0x8f */, + 1 /* 0x90 */, 1 /* 0x91 */, 1 /* 0x92 */, 1 /* 0x93 */, + 1 /* 0x94 */, 1 /* 0x95 */, 1 /* 0x96 */, 1 /* 0x97 */, + 1 /* 0x98 */, 1 /* 0x99 */, 1 /* 0x9a */, 1 /* 0x9b */, + 1 /* 0x9c */, 1 /* 0x9d */, 1 /* 0x9e */, 1 /* 0x9f */, + 1 /* 0xa0 */, 1 /* 0xa1 */, 1 /* 0xa2 */, 1 /* 0xa3 */, + 1 /* 0xa4 */, 1 /* 0xa5 */, 1 /* 0xa6 */, 1 /* 0xa7 */, + 1 /* 0xa8 */, 1 /* 0xa9 */, 1 /* 0xaa */, 1 /* 0xab */, + 1 /* 0xac */, 1 /* 0xad */, 1 /* 0xae */, 1 /* 0xaf */, + 1 /* 0xb0 */, 1 /* 0xb1 */, 1 /* 0xb2 */, 1 /* 0xb3 */, + 1 /* 0xb4 */, 1 /* 0xb5 */, 1 /* 0xb6 */, 1 /* 0xb7 */, + 1 /* 0xb8 */, 1 /* 0xb9 */, 1 /* 0xba */, 1 /* 0xbb */, + 1 /* 0xbc */, 1 /* 0xbd */, 1 /* 0xbe */, 1 /* 0xbf */, + 1 /* 0xc0 */, 1 /* 0xc1 */, 1 /* 0xc2 */, 1 /* 0xc3 */, + 1 /* 0xc4 */, 1 /* 0xc5 */, 1 /* 0xc6 */, 1 /* 0xc7 */, + 1 /* 0xc8 */, 1 /* 0xc9 */, 1 /* 0xca */, 1 /* 0xcb */, + 1 /* 0xcc */, 1 /* 0xcd */, 1 /* 0xce */, 1 /* 0xcf */, + 1 /* 0xd0 */, 1 /* 0xd1 */, 1 /* 0xd2 */, 1 /* 0xd3 */, + 1 /* 0xd4 */, 1 /* 0xd5 */, 1 /* 0xd6 */, 1 /* 0xd7 */, + 1 /* 0xd8 */, 1 /* 0xd9 */, 1 /* 0xda */, 1 /* 0xdb */, + 1 /* 0xdc */, 1 /* 0xdd */, 1 /* 0xde */, 1 /* 0xdf */, + 1 /* 0xe0 */, 1 /* 0xe1 */, 1 /* 0xe2 */, 1 /* 0xe3 */, + 1 /* 0xe4 */, 1 /* 0xe5 */, 1 /* 0xe6 */, 1 /* 0xe7 */, + 1 /* 0xe8 */, 1 /* 0xe9 */, 1 /* 0xea */, 1 /* 0xeb */, + 1 /* 0xec */, 1 /* 0xed */, 1 /* 0xee */, 1 /* 0xef */, + 1 /* 0xf0 */, 1 /* 0xf1 */, 1 /* 0xf2 */, 1 /* 0xf3 */, + 1 /* 0xf4 */, 1 /* 0xf5 */, 1 /* 0xf6 */, 1 /* 0xf7 */, + 1 /* 0xf8 */, 1 /* 0xf9 */, 1 /* 0xfa */, 1 /* 0xfb */, + 1 /* 0xfc */, 1 /* 0xfd */, 1 /* 0xfe */, 1 /* 0xff */ +}; + +int nghttp2_check_path(const uint8_t *value, size_t len) { + const uint8_t *last; + for (last = value + len; value != last; ++value) { + if (!VALID_PATH_CHARS[*value]) { + return 0; + } + } + return 1; +} + +/* Generated by genauthoritychartbl.py */ static char VALID_AUTHORITY_CHARS[] = { 0 /* NUL */, 0 /* SOH */, 0 /* STX */, 0 /* ETX */, 0 /* EOT */, 0 /* ENQ */, 0 /* ACK */, 0 /* BEL */, diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c index 62f57b6a..ab757f44 100644 --- a/lib/nghttp2_http.c +++ b/lib/nghttp2_http.c @@ -360,7 +360,13 @@ int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream, return NGHTTP2_ERR_IGN_HTTP_HEADER; } - if (nv->token == NGHTTP2_TOKEN__AUTHORITY || + if (nv->token == NGHTTP2_TOKEN__METHOD) { + rv = nghttp2_check_method(nv->value->base, nv->value->len); + } + else if (nv->token == NGHTTP2_TOKEN__PATH) { + rv = nghttp2_check_path(nv->value->base, nv->value->len); + } + else if (nv->token == NGHTTP2_TOKEN__AUTHORITY || nv->token == NGHTTP2_TOKEN_HOST) { rv = nghttp2_check_authority(nv->value->base, nv->value->len); } else if (nv->token == NGHTTP2_TOKEN__SCHEME) {