From 01da95feade5e5612499f5374498e7968c1a4a82 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Tue, 27 Feb 2018 22:59:00 +0100
Subject: [PATCH] Reject interior blanks in Content-Length value.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this commit `Content-Length: 4 2` was accepted as a valid header
and recorded as `parser->content_length = 42`.  Now it is a parse error
that fails with error `HPE_INVALID_CONTENT_LENGTH`.

Downstream users that inspect `parser->content_length` and naively parse
the string value using `strtoul()` might get confused by the discrepancy
between the two values.  Resolve that by simply not letting it happen.

Fixes: https://github.com/nodejs-private/security/issues/178
PR-URL: https://github.com/nodejs-private/http-parser-private/pull/1
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
---
 http_parser.c | 19 ++++++++++++++++++-
 test.c        | 21 +++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/http_parser.c b/http_parser.c
index 5b5657b..f9991c3 100644
--- a/http_parser.c
+++ b/http_parser.c
@@ -370,6 +370,8 @@ enum header_states
 
   , h_connection
   , h_content_length
+  , h_content_length_num
+  , h_content_length_ws
   , h_transfer_encoding
   , h_upgrade
 
@@ -1406,6 +1408,7 @@ reexecute:
 
             parser->flags |= F_CONTENTLENGTH;
             parser->content_length = ch - '0';
+            parser->header_state = h_content_length_num;
             break;
 
           case h_connection:
@@ -1493,10 +1496,18 @@ reexecute:
               break;
 
             case h_content_length:
+              if (ch == ' ') break;
+              h_state = h_content_length_num;
+              /* FALLTHROUGH */
+
+            case h_content_length_num:
             {
               uint64_t t;
 
-              if (ch == ' ') break;
+              if (ch == ' ') {
+                h_state = h_content_length_ws;
+                break;
+              }
 
               if (UNLIKELY(!IS_NUM(ch))) {
                 SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
@@ -1519,6 +1530,12 @@ reexecute:
               break;
             }
 
+            case h_content_length_ws:
+              if (ch == ' ') break;
+              SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
+              parser->header_state = h_state;
+              goto error;
+
             /* Transfer-Encoding: chunked */
             case h_matching_transfer_encoding_chunked:
               parser->index++;
diff --git a/test.c b/test.c
index 18b328d..df170e4 100644
--- a/test.c
+++ b/test.c
@@ -4147,6 +4147,27 @@ main (void)
   test_invalid_header_field_token_error(HTTP_RESPONSE);
   test_invalid_header_field_content_error(HTTP_RESPONSE);
 
+  test_simple_type(
+      "POST / HTTP/1.1\r\n"
+      "Content-Length:  42 \r\n"  // Note the surrounding whitespace.
+      "\r\n",
+      HPE_OK,
+      HTTP_REQUEST);
+
+  test_simple_type(
+      "POST / HTTP/1.1\r\n"
+      "Content-Length: 4 2\r\n"
+      "\r\n",
+      HPE_INVALID_CONTENT_LENGTH,
+      HTTP_REQUEST);
+
+  test_simple_type(
+      "POST / HTTP/1.1\r\n"
+      "Content-Length: 13 37\r\n"
+      "\r\n",
+      HPE_INVALID_CONTENT_LENGTH,
+      HTTP_REQUEST);
+
   //// RESPONSES
 
   test_simple_type("HTP/1.1 200 OK\r\n\r\n", HPE_INVALID_VERSION, HTTP_RESPONSE);