From 998bcc5a97972752fc1402cbb6ad915db31657f3 Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Tue, 12 Sep 2023 10:19:03 +0200 Subject: [PATCH 1/2] fix: Do not allow body for HTTP 304. --- src/native/http.c | 19 +++++-- test/response/connection.md | 103 +++++++++++++++++++++++++++++++----- 2 files changed, 106 insertions(+), 16 deletions(-) diff --git a/src/native/http.c b/src/native/http.c index 3a66044f..99eefb47 100644 --- a/src/native/http.c +++ b/src/native/http.c @@ -39,13 +39,26 @@ int llhttp__after_headers_complete(llhttp_t* parser, const char* p, int hasBody; hasBody = parser->flags & F_CHUNKED || parser->content_length > 0; - if (parser->upgrade && (parser->method == HTTP_CONNECT || - (parser->flags & F_SKIPBODY) || !hasBody)) { + if ( + (parser->upgrade && (parser->method == HTTP_CONNECT || + (parser->flags & F_SKIPBODY) || !hasBody)) || + /* See RFC 2616 section 4.4 - 1xx e.g. Continue */ + (parser->type == HTTP_RESPONSE && parser->status_code / 100 == 1) + ) { /* Exit, the rest of the message is in a different protocol. */ return 1; } - if (parser->flags & F_SKIPBODY) { + /* See RFC 2616 section 4.4 */ + if ( + parser->flags & F_SKIPBODY || /* response to a HEAD request */ + ( + parser->type == HTTP_RESPONSE && ( + parser->status_code == 204 || /* No Content */ + parser->status_code == 304 /* Not Modified */ + ) + ) + ) { return 0; } else if (parser->flags & F_CHUNKED) { /* chunked encoding - ignore Content-Length header, prepare for a chunk */ diff --git a/test/response/connection.md b/test/response/connection.md index d1d7bdd4..6294ab9e 100644 --- a/test/response/connection.md +++ b/test/response/connection.md @@ -298,9 +298,8 @@ off=84 header_field complete off=85 len=1 span[header_value]="4" off=88 header_value complete off=90 headers complete status=101 v=1/1 flags=34 content_length=4 -off=90 len=4 span[body]="body" -off=94 message complete -off=94 error code=22 reason="Pause on CONNECT/Upgrade" +off=90 message complete +off=90 error code=22 reason="Pause on CONNECT/Upgrade" ``` ## HTTP 101 response with Upgrade and Transfer-Encoding header @@ -340,16 +339,8 @@ off=87 header_field complete off=88 len=7 span[header_value]="chunked" off=97 header_value complete off=99 headers complete status=101 v=1/1 flags=21c content_length=0 -off=102 chunk header len=2 -off=102 len=2 span[body]="bo" -off=106 chunk complete -off=109 chunk header len=2 -off=109 len=2 span[body]="dy" -off=113 chunk complete -off=116 chunk header len=0 -off=118 chunk complete -off=118 message complete -off=118 error code=22 reason="Pause on CONNECT/Upgrade" +off=99 message complete +off=99 error code=22 reason="Pause on CONNECT/Upgrade" ``` ## HTTP 200 response with Upgrade header @@ -463,3 +454,89 @@ off=99 chunk header len=0 off=101 chunk complete off=101 message complete ``` + +## HTTP 304 with Content-Length + + +```http +HTTP/1.1 304 Not Modified +Content-Length: 10 + + +HTTP/1.1 200 OK +Content-Length: 5 + +hello +``` + +```log +off=0 message begin +off=5 len=3 span[version]="1.1" +off=8 version complete +off=13 len=12 span[status]="Not Modified" +off=27 status complete +off=27 len=14 span[header_field]="Content-Length" +off=42 header_field complete +off=43 len=2 span[header_value]="10" +off=47 header_value complete +off=49 headers complete status=304 v=1/1 flags=20 content_length=10 +off=49 message complete +off=51 reset +off=51 message begin +off=56 len=3 span[version]="1.1" +off=59 version complete +off=64 len=2 span[status]="OK" +off=68 status complete +off=68 len=14 span[header_field]="Content-Length" +off=83 header_field complete +off=84 len=1 span[header_value]="5" +off=87 header_value complete +off=89 headers complete status=200 v=1/1 flags=20 content_length=5 +off=89 len=5 span[body]="hello" +off=94 message complete +``` + +## HTTP 304 with Transfer-Encoding + + +```http +HTTP/1.1 304 Not Modified +Transfer-Encoding: chunked + +HTTP/1.1 200 OK +Transfer-Encoding: chunked + +5 +hello +0 + +``` + +```log +off=0 message begin +off=5 len=3 span[version]="1.1" +off=8 version complete +off=13 len=12 span[status]="Not Modified" +off=27 status complete +off=27 len=17 span[header_field]="Transfer-Encoding" +off=45 header_field complete +off=46 len=7 span[header_value]="chunked" +off=55 header_value complete +off=57 headers complete status=304 v=1/1 flags=208 content_length=0 +off=57 message complete +off=57 reset +off=57 message begin +off=62 len=3 span[version]="1.1" +off=65 version complete +off=70 len=2 span[status]="OK" +off=74 status complete +off=74 len=17 span[header_field]="Transfer-Encoding" +off=92 header_field complete +off=93 len=7 span[header_value]="chunked" +off=102 header_value complete +off=104 headers complete status=200 v=1/1 flags=208 content_length=0 +off=107 chunk header len=5 +off=107 len=5 span[body]="hello" +off=114 chunk complete +off=117 chunk header len=0 +``` From c3738682e5c04720ea339ecedfb0add22672b460 Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Tue, 12 Sep 2023 10:41:41 +0200 Subject: [PATCH 2/2] feat: Added new lenient flags to allow spaces after chunk header. --- README.md | 9 ++++ src/llhttp/constants.ts | 1 + src/llhttp/http.ts | 10 +++++ src/native/api.c | 8 ++++ test/fixtures/extra.c | 10 +++++ test/fixtures/index.ts | 3 ++ test/request/transfer-encoding.md | 67 +++++++++++++++++++++++++++++- test/response/transfer-encoding.md | 4 +- 8 files changed, 108 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 451a77f2..9700c32a 100644 --- a/README.md +++ b/README.md @@ -381,6 +381,15 @@ With this flag the new chunk can start immediately after the previous one. **Enabling this flag can pose a security issue since you will be exposed to request smuggling attacks. USE WITH CAUTION!** +### `void llhttp_set_lenient_spaces_after_chunk_size(llhttp_t* parser, int enabled)` + +Enables/disables lenient handling of spaces after chunk size. + +Normally `llhttp` would error when after a chunk size is followed by one or more spaces are present instead of a CRLF or `;`. +With this flag this check is disabled. + +**Enabling this flag can pose a security issue since you will be exposed to request smuggling attacks. USE WITH CAUTION!** + ## Build Instructions Make sure you have [Node.js](https://nodejs.org/), npm and npx installed. Then under project directory run: diff --git a/src/llhttp/constants.ts b/src/llhttp/constants.ts index 96aa31aa..bdf8afbe 100644 --- a/src/llhttp/constants.ts +++ b/src/llhttp/constants.ts @@ -74,6 +74,7 @@ export enum LENIENT_FLAGS { OPTIONAL_LF_AFTER_CR = 1 << 6, OPTIONAL_CRLF_AFTER_CHUNK = 1 << 7, OPTIONAL_CR_BEFORE_LF = 1 << 8, + SPACES_AFTER_CHUNK_SIZE = 1 << 9, } export enum METHODS { diff --git a/src/llhttp/http.ts b/src/llhttp/http.ts index be125c3d..83c6df80 100644 --- a/src/llhttp/http.ts +++ b/src/llhttp/http.ts @@ -924,6 +924,16 @@ export class HTTP { .otherwise(n('chunk_size_otherwise')); n('chunk_size_otherwise') + .match( + [ ' ', '\t' ], + this.testLenientFlags( + LENIENT_FLAGS.SPACES_AFTER_CHUNK_SIZE, + { + 1: n('chunk_size_otherwise'), + }, + p.error(ERROR.INVALID_CHUNK_SIZE, 'Invalid character in chunk size'), + ), + ) .match('\r', n('chunk_size_almost_done')) .match( '\n', diff --git a/src/native/api.c b/src/native/api.c index fa2133b7..8c4d008a 100644 --- a/src/native/api.c +++ b/src/native/api.c @@ -323,6 +323,14 @@ void llhttp_set_lenient_optional_cr_before_lf(llhttp_t* parser, int enabled) { } } +void llhttp_set_lenient_spaces_after_chunk_size(llhttp_t* parser, int enabled) { + if (enabled) { + parser->lenient_flags |= LENIENT_SPACES_AFTER_CHUNK_SIZE; + } else { + parser->lenient_flags &= ~LENIENT_SPACES_AFTER_CHUNK_SIZE; + } +} + /* Callbacks */ diff --git a/test/fixtures/extra.c b/test/fixtures/extra.c index 9b071873..dadf8dca 100644 --- a/test/fixtures/extra.c +++ b/test/fixtures/extra.c @@ -181,6 +181,16 @@ void llhttp__test_init_response_lenient_optional_crlf_after_chunk(llparse_t* s) s->lenient_flags |= LENIENT_OPTIONAL_CRLF_AFTER_CHUNK; } +void llhttp__test_init_request_lenient_spaces_after_chunk_size(llparse_t* s) { + llhttp__test_init_request(s); + s->lenient_flags |= LENIENT_SPACES_AFTER_CHUNK_SIZE; +} + +void llhttp__test_init_response_lenient_spaces_after_chunk_size(llparse_t* s) { + llhttp__test_init_response(s); + s->lenient_flags |= LENIENT_SPACES_AFTER_CHUNK_SIZE; +} + void llhttp__test_finish(llparse_t* s) { llparse__print(NULL, NULL, "finish=%d", s->finish); diff --git a/test/fixtures/index.ts b/test/fixtures/index.ts index 9dfa7b90..1571f9d3 100644 --- a/test/fixtures/index.ts +++ b/test/fixtures/index.ts @@ -20,6 +20,7 @@ export type TestType = 'request' | 'response' | 'request-finish' | 'response-fin 'request-lenient-optional-lf-after-cr' | 'response-lenient-optional-lf-after-cr' | 'request-lenient-optional-cr-before-lf' | 'response-lenient-optional-cr-before-lf' | 'request-lenient-optional-crlf-after-chunk' | 'response-lenient-optional-crlf-after-chunk' | + 'request-lenient-spaces-after-chunk-size' | 'response-lenient-spaces-after-chunk-size' | 'none' | 'url'; export const allowedTypes: TestType[] = [ @@ -45,6 +46,8 @@ export const allowedTypes: TestType[] = [ 'response-lenient-optional-cr-before-lf', 'request-lenient-optional-crlf-after-chunk', 'response-lenient-optional-crlf-after-chunk', + 'request-lenient-spaces-after-chunk-size', + 'response-lenient-spaces-after-chunk-size', ]; const BUILD_DIR = path.join(__dirname, '..', 'tmp'); diff --git a/test/request/transfer-encoding.md b/test/request/transfer-encoding.md index 19b2dec1..904c0092 100644 --- a/test/request/transfer-encoding.md +++ b/test/request/transfer-encoding.md @@ -302,7 +302,7 @@ off=83 header_field complete off=84 len=7 span[header_value]="chunked" off=93 header_value complete off=95 headers complete method=3 v=1/1 flags=208 content_length=0 -off=96 error code=12 reason="Invalid character in chunk size" +off=97 error code=12 reason="Invalid character in chunk size" ``` ### No extension after semicolon @@ -884,7 +884,7 @@ off=37 header_field complete off=38 len=7 span[header_value]="chunked" off=47 header_value complete off=49 headers complete method=4 v=1/1 flags=208 content_length=0 -off=50 error code=12 reason="Invalid character in chunk size" +off=51 error code=12 reason="Invalid character in chunk size" ``` ## Invalid OBS fold after chunked value @@ -1117,4 +1117,67 @@ off=79 chunk header len=5 off=79 len=5 span[body]="ABCDE" off=84 chunk complete off=87 chunk header len=0 +``` + +## Space after chunk header + + +```http +PUT /url HTTP/1.1 +Transfer-Encoding: chunked + +a \r\n0123456789 +0 + + +``` + +```log +off=0 message begin +off=0 len=3 span[method]="PUT" +off=3 method complete +off=4 len=4 span[url]="/url" +off=9 url complete +off=14 len=3 span[version]="1.1" +off=17 version complete +off=19 len=17 span[header_field]="Transfer-Encoding" +off=37 header_field complete +off=38 len=7 span[header_value]="chunked" +off=47 header_value complete +off=49 headers complete method=4 v=1/1 flags=208 content_length=0 +off=51 error code=12 reason="Invalid character in chunk size" +``` + +## Space after chunk header (lenient) + + +```http +PUT /url HTTP/1.1 +Transfer-Encoding: chunked + +a \r\n0123456789 +0 + + +``` + +```log +off=0 message begin +off=0 len=3 span[method]="PUT" +off=3 method complete +off=4 len=4 span[url]="/url" +off=9 url complete +off=14 len=3 span[version]="1.1" +off=17 version complete +off=19 len=17 span[header_field]="Transfer-Encoding" +off=37 header_field complete +off=38 len=7 span[header_value]="chunked" +off=47 header_value complete +off=49 headers complete method=4 v=1/1 flags=208 content_length=0 +off=53 chunk header len=10 +off=53 len=10 span[body]="0123456789" +off=65 chunk complete +off=68 chunk header len=0 +off=70 chunk complete +off=70 message complete ``` \ No newline at end of file diff --git a/test/response/transfer-encoding.md b/test/response/transfer-encoding.md index 6a2252e4..e1fd10ad 100644 --- a/test/response/transfer-encoding.md +++ b/test/response/transfer-encoding.md @@ -35,7 +35,7 @@ off=61 header_field complete off=62 len=7 span[header_value]="chunked" off=71 header_value complete off=73 headers complete status=200 v=1/1 flags=208 content_length=0 -off=75 error code=12 reason="Invalid character in chunk size" +off=76 error code=12 reason="Invalid character in chunk size" ``` ## `chunked` before other transfer-encoding @@ -229,7 +229,7 @@ off=52 header_field complete off=53 len=7 span[header_value]="chunked" off=62 header_value complete off=64 headers complete status=200 v=1/1 flags=208 content_length=0 -off=65 error code=12 reason="Invalid character in chunk size" +off=66 error code=12 reason="Invalid character in chunk size" ```