From 92146e00fd74890ec0e977c8f9592ddaae0314d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rimas=20Misevi=C4=8Dius?= Date: Mon, 2 Oct 2017 22:18:06 +0300 Subject: [PATCH] url: fix port overflow checking This patch adds (port > 0xffff) check after each digit in the loop and prevents integer overflow. PR-URL: https://github.com/nodejs/node/pull/15794 Refs: https://github.com/w3c/web-platform-tests/pull/7602 Reviewed-By: Anna Henningsen Reviewed-By: Daijiro Wachi Reviewed-By: Timothy Gu Reviewed-By: Colin Ihrig Reviewed-By: Joyee Cheung --- src/node_url.cc | 10 ++++++---- test/fixtures/url-tests.js | 20 +++++++++++++++++++- test/parallel/test-whatwg-url-parsing.js | 2 +- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/node_url.cc b/src/node_url.cc index 3dd89d74833..e7a0b47194e 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -1597,10 +1597,11 @@ void URL::Parse(const char* input, ch == '#' || special_back_slash) { if (buffer.size() > 0) { - int port = 0; - for (size_t i = 0; i < buffer.size(); i++) + unsigned port = 0; + // the condition port <= 0xffff prevents integer overflow + for (size_t i = 0; port <= 0xffff && i < buffer.size(); i++) port = port * 10 + buffer[i] - '0'; - if (port < 0 || port > 0xffff) { + if (port > 0xffff) { // TODO(TimothyGu): This hack is currently needed for the host // setter since it needs access to hostname if it is valid, and // if the FAILED flag is set the entire response to JS layer @@ -1611,7 +1612,8 @@ void URL::Parse(const char* input, url->flags |= URL_FLAGS_FAILED; return; } - url->port = NormalizePort(url->scheme, port); + // the port is valid + url->port = NormalizePort(url->scheme, static_cast(port)); buffer.clear(); } else if (has_state_override) { // TODO(TimothyGu): Similar case as above. diff --git a/test/fixtures/url-tests.js b/test/fixtures/url-tests.js index b4db586f447..48f77fe0774 100644 --- a/test/fixtures/url-tests.js +++ b/test/fixtures/url-tests.js @@ -2,7 +2,7 @@ /* The following tests are copied from WPT. Modifications to them should be upstreamed first. Refs: - https://github.com/w3c/web-platform-tests/blob/5d149f0/url/urltestdata.json + https://github.com/w3c/web-platform-tests/blob/11757f1/url/urltestdata.json License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html */ module.exports = @@ -5811,6 +5811,24 @@ module.exports = "base": "about:blank", "failure": true }, + "Port overflow (2^32 + 81)", + { + "input": "http://f:4294967377/c", + "base": "http://example.org/", + "failure": true + }, + "Port overflow (2^64 + 81)", + { + "input": "http://f:18446744073709551697/c", + "base": "http://example.org/", + "failure": true + }, + "Port overflow (2^128 + 81)", + { + "input": "http://f:340282366920938463463374607431768211537/c", + "base": "http://example.org/", + "failure": true + }, "# Non-special-URL path tests", { "input": "sc://ñ", diff --git a/test/parallel/test-whatwg-url-parsing.js b/test/parallel/test-whatwg-url-parsing.js index 00fedbde1b0..928ac5d6a6c 100644 --- a/test/parallel/test-whatwg-url-parsing.js +++ b/test/parallel/test-whatwg-url-parsing.js @@ -26,7 +26,7 @@ const failureTests = tests.filter((test) => test.failure).concat([ ]); const expectedError = common.expectsError( - { code: 'ERR_INVALID_URL', type: TypeError }, 110); + { code: 'ERR_INVALID_URL', type: TypeError }, failureTests.length); for (const test of failureTests) { assert.throws(