From 9ce7316de31f90d8485706a1ab8ef623404c2d8c Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor@indutny.com>
Date: Tue, 6 Feb 2018 17:33:16 -0500
Subject: [PATCH] src: fix out-of-bounds read through `strtoul`

`strtoul` will attempt to lookup the next digit up until it will stumble
upon an invalid one. However, for an unterminated string as an input
value, this results in out-of-bounds read.

Remove `strtoul` call, and replace it with simple loop.

Fix: #408
PR-URL: https://github.com/nodejs/http-parser/pull/409
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 http_parser.c | 28 +++++++++++++++++++++-------
 test.c        | 25 +++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/http_parser.c b/http_parser.c
index 8d85124..3d1e125 100644
--- a/http_parser.c
+++ b/http_parser.c
@@ -22,7 +22,6 @@
 #include <assert.h>
 #include <stddef.h>
 #include <ctype.h>
-#include <stdlib.h>
 #include <string.h>
 #include <limits.h>
 
@@ -2367,12 +2366,27 @@ http_parser_parse_url(const char *buf, size_t buflen, int is_connect,
   }
 
   if (u->field_set & (1 << UF_PORT)) {
-    /* Don't bother with endp; we've already validated the string */
-    unsigned long v = strtoul(buf + u->field_data[UF_PORT].off, NULL, 10);
-
-    /* Ports have a max value of 2^16 */
-    if (v > 0xffff) {
-      return 1;
+    uint16_t off;
+    uint16_t len;
+    const char* p;
+    const char* end;
+    unsigned long v;
+
+    off = u->field_data[UF_PORT].off;
+    len = u->field_data[UF_PORT].len;
+    end = buf + off + len;
+
+    /* NOTE: The characters are already validated and are in the [0-9] range */
+    assert(off + len <= buflen && "Port number overflow");
+    v = 0;
+    for (p = buf + off; p < end; p++) {
+      v *= 10;
+      v += *p - '0';
+
+      /* Ports have a max value of 2^16 */
+      if (v > 0xffff) {
+        return 1;
+      }
     }
 
     u->port = (uint16_t) v;
diff --git a/test.c b/test.c
index 193004e..34a5c45 100644
--- a/test.c
+++ b/test.c
@@ -3664,6 +3664,30 @@ test_header_cr_no_lf_error (int req)
   abort();
 }
 
+void
+test_no_overflow_parse_url (void)
+{
+  int rv;
+  struct http_parser_url u;
+
+  http_parser_url_init(&u);
+  rv = http_parser_parse_url("http://example.com:8001", 22, 0, &u);
+
+  if (rv != 0) {
+    fprintf(stderr,
+            "\n*** test_no_overflow_parse_url invalid return value=%d\n",
+            rv);
+    abort();
+  }
+
+  if (u.port != 800) {
+    fprintf(stderr,
+            "\n*** test_no_overflow_parse_url invalid port number=%d\n",
+            u.port);
+    abort();
+  }
+}
+
 void
 test_header_overflow_error (int req)
 {
@@ -4099,6 +4123,7 @@ main (void)
   test_header_nread_value();
 
   //// OVERFLOW CONDITIONS
+  test_no_overflow_parse_url();
 
   test_header_overflow_error(HTTP_REQUEST);
   test_no_overflow_long_body(HTTP_REQUEST, 1000);