Commit 70157b9
committed
url: forbid certain confusable changes from being introduced by toASCII
The legacy url.parse() function attempts to convert Unicode domains
(IDNs) into their ASCII/Punycode form through the use of the toASCII
function. However, toASCII can introduce or remove various characters
that at best invalidate the parsed URL, and at worst cause hostname
spoofing:
url.parse('http://bad.c℀.good.com/').href === 'http://bad.ca/c.good.com/'
(from [1])
url.parse('http://\u00AD/bad.com').href === 'http:///bad.com/'
While changes to the legacy URL parser are discouraged in general, the
security implications here outweigh the desire for strict compatibility.
This is since this commit only changes behavior when non-ASCII
characters appear in the hostname, an unusual situation for most use
cases. Additionally, despite the availability of the WHATWG URL API,
url.parse remain widely deployed in the Node.js ecosystem, as
exemplified by the recent un-deprecation of the legacy API.
This change is similar in spirit to CPython 3.8's change [2] fixing
bpo-36216 [3] aka CVE-2019-9636, which also occurred despite potential
compatibility concerns.
[1]: https://hackerone.com/reports/678487
[2]: python/cpython@16e6f7d
[3]: https://bugs.python.org/issue36216
PR-URL: #38631
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>1 parent 41fab0d commit 70157b9Copy full SHA for 70157b9
File tree
Expand file treeCollapse file tree
4 files changed
+74
-6
lines changedOpen diff view settings
Filter options
- doc/api
- lib
- test/parallel
Expand file treeCollapse file tree
4 files changed
+74
-6
lines changedOpen diff view settings
Collapse file
+5-4Lines changed: 5 additions & 4 deletions
- Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| ||
1677 | 1677 | |
1678 | 1678 | |
1679 | 1679 | |
1680 | | - |
1681 | | - |
1682 | | - |
1683 | | - |
| 1680 | + |
| 1681 | + |
| 1682 | + |
| 1683 | + |
1684 | 1684 | |
1685 | 1685 | |
1686 | 1686 | |
| ||
2824 | 2824 | |
2825 | 2825 | |
2826 | 2826 | |
| 2827 | + |
2827 | 2828 | |
2828 | 2829 | |
2829 | 2830 | |
|
Collapse file
+5Lines changed: 5 additions & 0 deletions
- Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| ||
1232 | 1232 | |
1233 | 1233 | |
1234 | 1234 | |
| 1235 | + |
| 1236 | + |
| 1237 | + |
| 1238 | + |
| 1239 | + |
1235 | 1240 | |
1236 | 1241 | |
1237 | 1242 | |
|
Collapse file
+30-2Lines changed: 30 additions & 2 deletions
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| ||
34 | 34 | |
35 | 35 | |
36 | 36 | |
37 | | - |
| 37 | + |
| 38 | + |
38 | 39 | |
39 | 40 | |
40 | 41 | |
| ||
167 | 168 | |
168 | 169 | |
169 | 170 | |
| 171 | + |
| 172 | + |
| 173 | + |
| 174 | + |
| 175 | + |
| 176 | + |
| 177 | + |
| 178 | + |
| 179 | + |
| 180 | + |
| 181 | + |
| 182 | + |
| 183 | + |
| 184 | + |
170 | 185 | |
171 | 186 | |
172 | 187 | |
| ||
389 | 404 | |
390 | 405 | |
391 | 406 | |
392 | | - |
| 407 | + |
393 | 408 | |
394 | 409 | |
395 | 410 | |
| ||
398 | 413 | |
399 | 414 | |
400 | 415 | |
| 416 | + |
| 417 | + |
| 418 | + |
| 419 | + |
| 420 | + |
| 421 | + |
| 422 | + |
| 423 | + |
| 424 | + |
| 425 | + |
| 426 | + |
| 427 | + |
| 428 | + |
401 | 429 | |
402 | 430 | |
403 | 431 | |
|
Collapse file
test/parallel/test-url-parse-invalid-input.js
Copy file name to clipboardExpand all lines: test/parallel/test-url-parse-invalid-input.js+34Lines changed: 34 additions & 0 deletions
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| ||
36 | 36 | |
37 | 37 | |
38 | 38 | |
| 39 | + |
| 40 | + |
| 41 | + |
| 42 | + |
| 43 | + |
| 44 | + |
| 45 | + |
| 46 | + |
| 47 | + |
| 48 | + |
| 49 | + |
| 50 | + |
| 51 | + |
| 52 | + |
| 53 | + |
| 54 | + |
| 55 | + |
| 56 | + |
| 57 | + |
| 58 | + |
| 59 | + |
| 60 | + |
| 61 | + |
| 62 | + |
| 63 | + |
| 64 | + |
| 65 | + |
| 66 | + |
| 67 | + |
| 68 | + |
| 69 | + |
| 70 | + |
| 71 | + |
| 72 | + |
0 commit comments