Fix: Potential Vulnerability in HTTP Parser Implementation

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

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) {

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 @@ size_t http_parser_execute (http_parser *parser,
       {
         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 @@ size_t http_parser_execute (http_parser *parser,
       {
         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);
@@ -2858,8 +2861,14 @@ size_t http_parser_execute (http_parser *parser,
           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: {
@@ -2941,6 +2950,14 @@ size_t http_parser_execute (http_parser *parser,
                 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 +3388,23 @@ size_t http_parser_execute (http_parser *parser,
           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 +3478,31 @@ size_t http_parser_execute (http_parser *parser,
           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 */