1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

url: fix port overflow checking

Browse Source 
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 <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>
This commit is contained in:
Rimas Misevičius 2017-10-02 22:18:06 +03:00 committed by Timothy Gu
parent 85a5a2c228
commit 92146e00fd
No known key found for this signature in database
GPG Key ID: 7FE6B095B582B0D4
3 changed files with 26 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/node_url.cc
View File

@ -1597,10 +1597,11 @@ void URL::Parse(const char* input,
ch == '#' || ch == '#' ||
special_back_slash) { special_back_slash) {
if (buffer.size() > 0) { if (buffer.size() > 0) {
int port = 0; unsigned port = 0;
for (size_t i = 0; i < buffer.size(); i++) // the condition port <= 0xffff prevents integer overflow
for (size_t i = 0; port <= 0xffff && i < buffer.size(); i++)
port = port * 10 + buffer[i] - '0'; port = port * 10 + buffer[i] - '0';
if (port < 0 || port > 0xffff) { if (port > 0xffff) {
// TODO(TimothyGu): This hack is currently needed for the host // TODO(TimothyGu): This hack is currently needed for the host
// setter since it needs access to hostname if it is valid, and // setter since it needs access to hostname if it is valid, and
// if the FAILED flag is set the entire response to JS layer // 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; url->flags |= URL_FLAGS_FAILED;
return; return;
} }
url->port = NormalizePort(url->scheme, port); // the port is valid
url->port = NormalizePort(url->scheme, static_cast<int>(port));
buffer.clear(); buffer.clear();
} else if (has_state_override) { } else if (has_state_override) {
// TODO(TimothyGu): Similar case as above. // TODO(TimothyGu): Similar case as above.

20
test/fixtures/url-tests.js vendored
View File

@ -2,7 +2,7 @@
/* The following tests are copied from WPT. Modifications to them should be /* The following tests are copied from WPT. Modifications to them should be
upstreamed first. Refs: 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 License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html
*/ */
module.exports = module.exports =
@ -5811,6 +5811,24 @@ module.exports =
"base": "about:blank", "base": "about:blank",
"failure": true "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", "# Non-special-URL path tests",
{ {
"input": "sc://ñ", "input": "sc://ñ",

2
test/parallel/test-whatwg-url-parsing.js
View File

@ -26,7 +26,7 @@ const failureTests = tests.filter((test) => test.failure).concat([
]); ]);
const expectedError = common.expectsError( const expectedError = common.expectsError(
{ code: 'ERR_INVALID_URL', type: TypeError }, 110); { code: 'ERR_INVALID_URL', type: TypeError }, failureTests.length);
for (const test of failureTests) { for (const test of failureTests) {
assert.throws( assert.throws(