Repository navigation
Tweak security disclaimer regarding legacy url.parse #45311
Description
Affected URL(s)
https://nodejs.org/api/url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost
Description of the problem
I know there is a bit of history regarding deprecating, undeprecating, legacy, doc deprecating, etc., which I'm not looking to rehash. My question is specific to a small tweak or clarification on the last paragraph that mentions security. The current docs have the following warning:
It is prone to security issues such as host name spoofing and incorrect handling of usernames and passwords. Do not use with untrusted input. CVEs are not issued for
url.parse()vulnerabilities. Use the WHATWG URL API instead.
Is it possible to clarify there are no active security vulnerabilities with it. Perhaps it's just me, but every time I read that paragraph (even though I know the specific vulnerability called out has indeed been fixed in #38631), I always do a double take because it looks like the docs are saying there are active vulnerabilities that won't be fixed.
new URL() is still about 50% slower than url.parse in v19, and url.parse is still heavily used in the wild, so I think it'd be a value add in terms of clarity, but let me know your thoughts. I'm not sure exactly what the wording should be, but just making it clearer that it's not an active security risk unless the current wording is intended to scare people away 😆
Activity
anonrig commentedon Nov 4, 2022
CC @nodejs/security @nodejs/url
mcollina commentedon Nov 4, 2022
Here is how it should read: it's unsafe to use with untrusted inputs. There are edge cases that could potentially be exploited. The algorithm does not follow the spec, so there are an indefinite amount of possible attacks.
Fixing the vulnerabilities would be breaking changes or will make it as slow as new URL().
There are also valid use cases for this API, i.e. parsing the connection string of a database driver, or an URL from a config file.
aduh95 commentedon Nov 5, 2022
@mmarti would you like to open a PR to make those clarifications?
mcollina commentedon Nov 5, 2022
This includes performing HTTP requests or any other meaningful uses of URL.parse().
The most important part of that phrase is about the input: if you do not trust the input, do not expect url.parse() to be correct, spec compliant and secure. In other terms, if you or any other members of your team did not write the URL you are parsing... DO NOT USE url.parse().