8000 Reject some messages a conformant sender would not have sent by DemiMarie · Pull Request #728 · nginx/nginx · GitHub
[go: up one dir, main page]

Skip to content

Reject some messages a conformant sender would not have sent #728

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from
Draft
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
80b41e1
HTTP: Use common header validation function for HTTP/2 and HTTP/3
DemiMarie Mar 25, 2025
3897c97
Strip leading and trailing whitespace from HTTP field values
DemiMarie Apr 7, 2025
c7d26cc
HTTP/3: Do not allow invalid pseudo-header fields
DemiMarie Mar 21, 2025
17ce4bc
HTTP: Use 8000 common header code for v2 and v3
DemiMarie Mar 22, 2025
1e30988
HTTP: Reject HTTP/2 and HTTP/3 requests with Transfer-Encoding
DemiMarie Apr 8, 2025
b1c4b07
HTTP: Reject invalid header names
DemiMarie Mar 14, 2025
ae76c64
HTTP: Reject invalid field values
DemiMarie Apr 9, 2025
279ae48
HTTP: Reject hop-by-hop headers in HTTP/2 and HTTP/3 requests
DemiMarie Mar 13, 2025
98d2669
Proxy: Reject Transfer-Encoding or Content-Length trailers
DemiMarie Mar 27, 2025
dab8c0e
HTTP: Do not allow header lines with no colon
DemiMarie Apr 25, 2025
e30ddb7
HTTP: Do not allow multiple CRs before LF
DemiMarie Mar 14, 2025
0d2c875
HTTP: Do not allow request lines to end with bare CR
DemiMarie Mar 16, 2025
1be6976
Reject malformed chunk extensions
DemiMarie Mar 23, 2025
1c0426c
Forbid malformed HTTP/1.1 trailers
DemiMarie Mar 27, 2025
626b1d9
HTTP: Reject trailers involved in framing
DemiMarie Mar 27, 2025
436282f
HTTP: Remove redundant state from chunk parsing
DemiMarie Apr 7, 2025
bb0a2b9
HTTP: Allow rejecting leading and trailing whitespace in HTTP2+ fields
DemiMarie Mar 25, 2025
bd37faf
HTTP: do not allow headers to end with a bare LF
DemiMarie Mar 16, 2025
7032a60
HTTP: do not allow status line to end with bare LF
DemiMarie Mar 16, 2025
9950dfb
Do not allow trailers to end with bare LF
DemiMarie Jun 10, 2025
9b8e4a1
Do not support bad whitespace in chunk extensions
DemiMarie Jun 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Reject malformed chunk extensions
This forbids chunk extensions that violate RFC9112, and _only_ these
chunk extensions.  Bad whitespace is permitted, but a bare LF instead of
CRLF is not.
  • Loading branch information
DemiMarie committed Jun 10, 2025
commit 1be6976866f98f205bdab4f7f454b7c2a1762d6f
229 changes: 147 additions & 82 deletions src/http/ngx_http_parse.c
10000
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ static uint32_t usual[] = {

#endif

static inline ngx_int_t
ngx_http_field_value_char(u_char ch)
{
return ch >= 0x20 ? ch != 0x7f : ch == 0x09;
}

/* gcc, icc, msvc and others compile these switches as an jump table */

Expand Down Expand Up @@ -818,6 +823,17 @@ ngx_http_non_alnum_dash_header_char(u_char ch)
}
}

static ngx_int_t
ngx_http_token_char(u_char ch)
{
u_char c = (ch | 0x20);
if (('a' <= c && c <= 'z') || ('0' <= c && c <= '9') || c == '-') {
return 1;
}

return ngx_http_non_alnum_dash_header_char(ch);
}

ngx_int_t
ngx_http_parse_header_line(ngx_http_request_t *r, ngx_buf_t *b,
ngx_uint_t allow_underscores)
Expand Down Expand Up @@ -1117,9 +1133,7 @@ ngx_http_v23_fixup_header(ngx_http_request_t *r, ngx_str_t *name,
}

for (i = 0; i != value->len; i++) {
ch = value->data[i];

if (ch < 0x20 ? ch != 0x09 : ch == 0x7f) {
if (!ngx_http_field_value_char(value->data[i])) {
ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
"client sent header \"%V\" with "
"invalid value: \"%V\"",
Expand Down Expand Up @@ -1916,6 +1930,11 @@ ngx_http_parse_status_line(ngx_http_request_t *r, ngx_buf_t *b,
break;
case LF:
goto done;
default:
if (ch < 0x20 || ch == 0x7f) {
return NGX_ERROR;
}
break;
}
break;

Expand Down Expand Up @@ -2263,13 +2282,18 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
enum {
sw_chunk_start = 0,
sw_chunk_size,
sw_chunk_extension_before_semi,
sw_chunk_extension,
sw_chunk_extension_bws_before_equal,
sw_chunk_extension_name,
sw_chunk_extension_value_start,
sw_chunk_extension_quoted_value,
sw_chunk_extension_value_quoted_backslash,
sw_chunk_extension_unquoted_value,
sw_chunk_extension_almost_done,
sw_chunk_data,
sw_after_data,
sw_after_data_almost_done,
sw_last_chunk_extension,
sw_last_chunk_extension_almost_done,
sw_trailer,
sw_trailer_almost_done,
sw_trailer_header,
Expand Down Expand Up @@ -2326,105 +2350,116 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
ctx->size = ctx->size * 16 + (c - 'a' + 10);
break;
}
/* fall through */

if (ctx->size == 0) {

switch (ch) {
case CR:
state = sw_last_chunk_extension_almost_done;
break;
case LF:
if (keep_trailers) {
goto done;
}
state = sw_trailer;
break;
case ';':
case ' ':
case '\t':
state = sw_last_chunk_extension;
break;
default:
goto invalid;
}

break;
}

case sw_chunk_extension_before_semi:
before_semi:
switch (ch) {
case CR:
state = sw_chunk_extension_almost_done;
break;
case LF:
state = sw_chunk_data;
break;
case ';':
case ' ':
case '\t':
state = sw_chunk_extension;
break;
case ' ':
case '\t':
/*
* This switch is also used by other states, so set
* the state explicitly here.
*/
state = sw_chunk_extension_before_semi;
break; /* BWS */
default:
goto invalid;
}

break;

case sw_chunk_extension:
switch (ch) {
case CR:
state = sw_chunk_extension_almost_done;
if (ngx_http_token_char(ch)) {
state = sw_chunk_extension_name;
break;
case LF:
state = sw_chunk_data;
}
switch (ch) {
case ' ':
case '\t':
break; /* BWS */
default:
goto invalid;
}
break;

case sw_chunk_extension_almost_done:
if (ch == LF) {
state = sw_chunk_data;
case sw_chunk_extension_name:
if (ngx_http_token_char(ch)) {
break;
}
goto invalid;

case sw_chunk_data:
rc = NGX_OK;
goto data;
state = sw_chunk_extension_bws_before_equal;

case sw_after_data:
/* fall through */

case sw_chunk_extension_bws_before_equal:
switch (ch) {
case CR:
state = sw_after_data_almost_done;
case ' ':
case '\t':
break; /* BWS */
case '=':
state = sw_chunk_extension_value_start;
break;
case LF:
state = sw_chunk_start;
default:
goto invalid;
}
break;

case sw_chunk_extension_value_start:
if (ngx_http_token_char(ch)) {
state = sw_chunk_extension_unquoted_value;
break;
}
switch (ch) {
case ' ':
case '\t':
break; /* BWS */
case '"':
state = sw_chunk_extension_quoted_value;
break;
default:
goto invalid;
}
break;

case sw_after_data_almost_done:
if (ch == LF) {
state = sw_chunk_start;
case sw_chunk_extension_quoted_value:
if (ch == '"') {
state = sw_chunk_extension_before_semi;
break;
}
if (ch == '\\') {
state = sw_chunk_extension_value_quoted_backslash;
break;
}
if (ngx_http_field_value_char(ch)) {
break;
}
goto invalid;

case sw_last_chunk_extension:
switch (ch) {
case CR:
state = sw_last_chunk_extension_almost_done;
case sw_chunk_extension_value_quoted_backslash:
if (ngx_http_field_value_char(ch)) {
state = sw_chunk_extension_quoted_value;
break;
case LF:
if (keep_trailers) {
goto done;
}
state = sw_trailer;
}
break;
goto invalid;

case sw_chunk_extension_unquoted_value:
if (ngx_http_token_char(ch)) {
break;
}
goto before_semi;

case sw_last_chunk_extension_almost_done:
case sw_chunk_extension_almost_done:
if (ch == LF) {
if (ctx->size) {
state = sw_chunk_data;
break;
}
if (keep_trailers) {
goto done;
}
Expand All @@ -2433,6 +2468,24 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
}
goto invalid;

case sw_chunk_data:
rc = NGX_OK;
goto data;

case sw_after_data:
if (ch == CR) {
state = sw_after_data_almost_done;
break;
}
goto invalid;

case sw_after_data_almost_done:
if (ch == LF) {
state = sw_chunk_start;
break;
}
goto invalid;

case sw_trailer:
switch (ch) {
case CR:
Expand Down Expand Up @@ -2476,34 +2529,47 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
ctx->state = state;
b->pos = pos;

if (ctx->size > NGX_MAX_OFF_T_VALUE - 5) {
if (ctx->size > NGX_MAX_OFF_T_VALUE - 11) {
goto invalid;
}

off_t min_length = (ctx->size ? ctx->size + 6 /* CR LF "0" CR LF LF */
: 1 /* LF */);
switch (state) {

case sw_chunk_start:
ctx->length = 3 /* "0" LF LF */;
ctx->length = 4 /* "0" CR LF LF */;
break;
case sw_chunk_size:
ctx->length = 1 /* LF */
A3D4 + (ctx->size ? ctx->size + 4 /* LF "0" LF LF */
: 1 /* LF */);
case sw_chunk_extension_before_semi:
case sw_chunk_extension_unquoted_value:
ctx->length = 2 /* CR LF */ + min_length;
break;
case sw_chunk_extension:
case sw_chunk_extension_almost_done:
ctx->length = 1 /* LF */ + ctx->size + 4 /* LF "0" LF LF */;
ctx->length = 1 /* LF */ + min_length;
break;
case sw_chunk_extension:
ctx->length = 5 /* a=b CR LF */ + min_length;
break;
case sw_chunk_extension_bws_before_equal:
case sw_chunk_extension_name:
ctx->length = 4 /* =b CR LF */ + min_length;
break;
case sw_chunk_extension_value_start:
ctx->length = 3 /* b CR LF */ + min_length;
break;
case sw_chunk_extension_quoted_value:
ctx->length = 3 /* " CR LF */ + min_length;
break;
case sw_chunk_extension_value_quoted_backslash:
ctx->length = 4 /* a" CR LF */ + min_length;
break;
case sw_chunk_data:
ctx->length = ctx->size + 4 /* LF "0" LF LF */;
ctx->length = min_length;
break;
case sw_after_data:
case sw_after_data_almost_done:
ctx->length = 4 /* LF "0" LF LF */;
ctx->length = 6 /* CR LF "0" CR LF LF */;
break;
case sw_last_chunk_extension:
case sw_last_chunk_extension_almost_done:
ctx->length = 2 /* LF LF */;
case sw_after_data_almost_done:
ctx->length = 5 /* LF "0" CR LF LF */;
break;
case sw_trailer:
case sw_trailer_almost_done:
Expand All @@ -2513,7 +2579,6 @@ ngx_http_parse_chunked(ngx_http_request_t *r, ngx_buf_t *b,
case sw_trailer_header_almost_done:
ctx->length = 2 /* LF LF */;
break;

}

return rc;
Expand Down
0