From 133795193a2c0aaebc42933ee654f064624afc80 Mon Sep 17 00:00:00 2001 From: yannaingtun Date: Fri, 28 Feb 2025 00:12:14 +0800 Subject: [PATCH 1/4] Fix HTTP parser vulnerability to prevent request smuggling This patch addresses an HTTP request smuggling vulnerability by: 1. Adding allow_chunked_length parameter 2. Properly handling multiple Transfer-Encoding headers 3. Implementing RFC 7230 Section 3.3.3 checks for Transfer-Encoding and Content-Length conflicts Based on fix: https://github.com/nodejs/node/commit/fc70ce08f5818a286fb5899a1bc3aff5965a745e --- .../src/protocol/srs_protocol_http_stack.cpp | 68 +++++++++++++++---- 1 file changed, 56 insertions(+), 12 deletions(-) diff --git a/trunk/src/protocol/srs_protocol_http_stack.cpp b/trunk/src/protocol/srs_protocol_http_stack.cpp index d6c2d2907..0c09f572c 100644 --- a/trunk/src/protocol/srs_protocol_http_stack.cpp +++ b/trunk/src/protocol/srs_protocol_http_stack.cpp @@ -2200,6 +2200,7 @@ size_t http_parser_execute (http_parser *parser, const char *status_mark = 0; enum state p_state = (enum state) parser->state; const unsigned int lenient = parser->lenient_http_headers; + const unsigned int allow_chunked_length = parser->allow_chunked_length; uint32_t nread = parser->nread; /* We're in an error state. Don't bother doing anything. */ @@ -2319,8 +2320,9 @@ reexecute: { if (ch == CR || ch == LF) break; - parser->flags = 0; - parser->content_length = ULLONG_MAX; + parser->flags = 0; + parser->uses_transfer_encoding = 0; + parser->content_length = ULLONG_MAX; if (ch == 'H') { UPDATE_STATE(s_res_H); @@ -2496,8 +2498,9 @@ reexecute: { if (ch == CR || ch == LF) break; - parser->flags = 0; - parser->content_length = ULLONG_MAX; + parser->flags = 0; + parser->uses_transfer_encoding = 0; + parser->content_length = ULLONG_MAX; if (UNLIKELY(!IS_ALPHA(ch))) { SET_ERRNO(HPE_INVALID_METHOD); @@ -2941,6 +2944,14 @@ reexecute: parser->header_state = h_general; } else if (parser->index == sizeof(TRANSFER_ENCODING)-2) { parser->header_state = h_transfer_encoding; + parser->uses_transfer_encoding = 1; + + /* Multiple `Transfer-Encoding` headers should be treated as + * one, but with values separate by a comma. + * + * See: https://tools.ietf.org/html/rfc7230#section-3.2.2 + */ + parser->flags &= ~F_CHUNKED; } break; @@ -3371,13 +3382,23 @@ reexecute: REEXECUTE(); } - /* Cannot use chunked encoding and a content-length header together - per the HTTP specification. */ - if ((parser->flags & F_CHUNKED) && - (parser->flags & F_CONTENTLENGTH)) { - SET_ERRNO(HPE_UNEXPECTED_CONTENT_LENGTH); - goto error; - } + /* Cannot use transfer-encoding and a content-length header together + per the HTTP specification. (RFC 7230 Section 3.3.3) */ + if ((parser->uses_transfer_encoding == 1) && + (parser->flags & F_CONTENTLENGTH)) { + /* Allow it for lenient parsing as long as `Transfer-Encoding` is + * not `chunked` or allow_length_with_encoding is set + */ + if (parser->flags & F_CHUNKED) { + if (!allow_chunked_length) { + SET_ERRNO(HPE_UNEXPECTED_CONTENT_LENGTH); + goto error; + } + } else if (!lenient) { + SET_ERRNO(HPE_UNEXPECTED_CONTENT_LENGTH); + goto error; + } + } UPDATE_STATE(s_headers_done); @@ -3451,8 +3472,31 @@ reexecute: UPDATE_STATE(NEW_MESSAGE()); CALLBACK_NOTIFY(message_complete); } else if (parser->flags & F_CHUNKED) { - /* chunked encoding - ignore Content-Length header */ + /* chunked encoding - ignore Content-Length header, + * prepare for a chunk */ UPDATE_STATE(s_chunk_size_start); + } else if (parser->uses_transfer_encoding == 1) { + if (parser->type == HTTP_REQUEST && !lenient) { + /* RFC 7230 3.3.3 */ + + /* If a Transfer-Encoding header field + * is present in a request and the chunked transfer coding is not + * the final encoding, the message body length cannot be determined + * reliably; the server MUST respond with the 400 (Bad Request) + * status code and then close the connection. + */ + SET_ERRNO(HPE_INVALID_TRANSFER_ENCODING); + RETURN(p - data); /* Error */ + } else { + /* RFC 7230 3.3.3 */ + + /* If a Transfer-Encoding header field is present in a response and + * the chunked transfer coding is not the final encoding, the + * message body length is determined by reading the connection until + * it is closed by the server. + */ + UPDATE_STATE(s_body_identity_eof); + } } else { if (parser->content_length == 0) { /* Content-Length header given but zero: Content-Length: 0\r\n */ From b7766637d4b4a7e926e5eeb4e3d028a44a0a37dd Mon Sep 17 00:00:00 2001 From: Yan Naing Tun Date: Tue, 4 Mar 2025 11:31:22 +0800 Subject: [PATCH 2/4] Update json2.js --- trunk/research/players/js/json2.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/trunk/research/players/js/json2.js b/trunk/research/players/js/json2.js index d89ecc7a2..a0116ae62 100755 --- a/trunk/research/players/js/json2.js +++ b/trunk/research/players/js/json2.js @@ -206,6 +206,8 @@ if (typeof JSON !== 'object') { '\\': '\\\\' }, rep; + // Locate the pattern definition for verbosity and update it + var verbosityPattern = "^(off|errors|warnings|(info|progress)|(debug|progress\\+)|(trace|progress\\+\\+)|progress\\+\\+\\+)$"; function quote(string) { From 8b5741882b6e6daa80d74b9bed822d279b1a92ed Mon Sep 17 00:00:00 2001 From: Yan Naing Tun Date: Tue, 4 Mar 2025 11:43:54 +0800 Subject: [PATCH 3/4] Update CMakeLists.txt --- trunk/ide/srs_clion/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/trunk/ide/srs_clion/CMakeLists.txt b/trunk/ide/srs_clion/CMakeLists.txt index 63cb7fd7a..0f198eba7 100755 --- a/trunk/ide/srs_clion/CMakeLists.txt +++ b/trunk/ide/srs_clion/CMakeLists.txt @@ -90,6 +90,10 @@ TARGET_LINK_LIBRARIES(srs -ldl -pthread) TARGET_LINK_LIBRARIES(srs -rdynamic) TARGET_LINK_LIBRARIES(srs -fsanitize=address -fno-omit-frame-pointer) +# Ensure pthread is correctly linked +find_package(Threads REQUIRED) +target_link_libraries(srs Threads::Threads) + ########################################################### # For utest. # See https://google.github.io/googletest/quickstart-cmake.html From f6238c2fc39e1b55ecb7e6a837d7533b6b51d6e8 Mon Sep 17 00:00:00 2001 From: Yan Naing Tun Date: Tue, 4 Mar 2025 12:49:10 +0800 Subject: [PATCH 4/4] Update srs_protocol_http_stack.cpp --- trunk/src/protocol/srs_protocol_http_stack.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/trunk/src/protocol/srs_protocol_http_stack.cpp b/trunk/src/protocol/srs_protocol_http_stack.cpp index 0c09f572c..876016c0a 100644 --- a/trunk/src/protocol/srs_protocol_http_stack.cpp +++ b/trunk/src/protocol/srs_protocol_http_stack.cpp @@ -2861,8 +2861,14 @@ reexecute: ch = *p; c = TOKEN(ch); - if (!c) - break; + if (!c) { + // Add this fix to explicitly reject space in header field names + if (ch == ' ') { + SET_ERRNO(HPE_INVALID_HEADER_TOKEN); + goto error; + } + break; + } switch (parser->header_state) { case h_general: {