Skip to content

Commit f5e56aa

Browse files
rmisevtargos
authored andcommitted
url: fix port overflow checking
This patch adds (port > 0xffff) check after each digit in the loop and prevents integer overflow. PR-URL: #15794 Refs: web-platform-tests/wpt#7602 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 497bceb commit f5e56aa

File tree

3 files changed

+26
-6
lines changed

3 files changed

+26
-6
lines changed

src/node_url.cc

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,10 +1597,11 @@ void URL::Parse(const char* input,
15971597
ch == '#' ||
15981598
special_back_slash) {
15991599
if (buffer.size() > 0) {
1600-
int port = 0;
1601-
for (size_t i = 0; i < buffer.size(); i++)
1600+
unsigned port = 0;
1601+
// the condition port <= 0xffff prevents integer overflow
1602+
for (size_t i = 0; port <= 0xffff && i < buffer.size(); i++)
16021603
port = port * 10 + buffer[i] - '0';
1603-
if (port < 0 || port > 0xffff) {
1604+
if (port > 0xffff) {
16041605
// TODO(TimothyGu): This hack is currently needed for the host
16051606
// setter since it needs access to hostname if it is valid, and
16061607
// if the FAILED flag is set the entire response to JS layer
@@ -1611,7 +1612,8 @@ void URL::Parse(const char* input,
16111612
url->flags |= URL_FLAGS_FAILED;
16121613
return;
16131614
}
1614-
url->port = NormalizePort(url->scheme, port);
1615+
// the port is valid
1616+
url->port = NormalizePort(url->scheme, static_cast<int>(port));
16151617
buffer.clear();
16161618
} else if (has_state_override) {
16171619
// TODO(TimothyGu): Similar case as above.

test/fixtures/url-tests.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
/* The following tests are copied from WPT. Modifications to them should be
44
upstreamed first. Refs:
5-
https://github.com/w3c/web-platform-tests/blob/5d149f0/url/urltestdata.json
5+
https://github.com/w3c/web-platform-tests/blob/11757f1/url/urltestdata.json
66
License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html
77
*/
88
module.exports =
@@ -5811,6 +5811,24 @@ module.exports =
58115811
"base": "about:blank",
58125812
"failure": true
58135813
},
5814+
"Port overflow (2^32 + 81)",
5815+
{
5816+
"input": "http://f:4294967377/c",
5817+
"base": "http://example.org/",
5818+
"failure": true
5819+
},
5820+
"Port overflow (2^64 + 81)",
5821+
{
5822+
"input": "http://f:18446744073709551697/c",
5823+
"base": "http://example.org/",
5824+
"failure": true
5825+
},
5826+
"Port overflow (2^128 + 81)",
5827+
{
5828+
"input": "http://f:340282366920938463463374607431768211537/c",
5829+
"base": "http://example.org/",
5830+
"failure": true
5831+
},
58145832
"# Non-special-URL path tests",
58155833
{
58165834
"input": "sc://ñ",

test/parallel/test-whatwg-url-parsing.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const failureTests = tests.filter((test) => test.failure).concat([
2626
]);
2727

2828
const expectedError = common.expectsError(
29-
{ code: 'ERR_INVALID_URL', type: TypeError }, 110);
29+
{ code: 'ERR_INVALID_URL', type: TypeError }, failureTests.length);
3030

3131
for (const test of failureTests) {
3232
assert.throws(

0 commit comments

Comments
 (0)