Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

security implications of legacy url.parse() should be more clearly documented #31279

Closed
sam-github opened this issue Jan 9, 2020 · 1 comment
Closed

security implications of legacy url.parse() should be more clearly documented #31279

sam-github opened this issue Jan 9, 2020 · 1 comment
Labels
discuss doc security url

Comments

@sam-github
Copy link
Contributor

@sam-github sam-github commented Jan 9, 2020 •
edited

  • Version: all
  • Platform: all
  • Subsystem: url

url.parse() is "sloppy" with its parsing, so use of it can result in behaviour unexpected by some users that has security implications.

It is marked as deprecated at https://nodejs.org/api/url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost, but the docs don't specifically call out the security issues, so people won't necessarily know that security is a reason to avoid it.

It also doesn't list the specific (known) security issues, so that its not possible for users of the legacy url.parse() API to determine whether their usage is insecure.

These should be addressed through documentation.

Related

Vulnerability reports in process of disclosure, so link will be dead for a while longer.
@sam-github
Copy link
Contributor Author

@sam-github sam-github commented Jan 10, 2020

https://hackerone.com/reports/678487 has been disclosed.

It would be helpful if the documentation was updated to warn about issues such as that.

jasnell added a commit to jasnell/node that referenced this issue Jul 6, 2020
@jasnell
doc: document security issues with url.parse()
061d486 
Fixes: nodejs#31279
@jasnell jasnell mentioned this issue Jul 6, 2020
Closed
3 tasks
MylesBorins added a commit that referenced this issue Jul 14, 2020
@jasnell @MylesBorins
doc: document security issues with url.parse()
b403d60 
Fixes: #31279

PR-URL: #34226
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins added a commit that referenced this issue Jul 16, 2020
@jasnell @MylesBorins
doc: document security issues with url.parse()
a6a656a 
Fixes: #31279

PR-URL: #34226
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
addaleax added a commit that referenced this issue Sep 22, 2020
@jasnell @addaleax
doc: document security issues with url.parse()
fc94e6f 
Fixes: #31279

PR-URL: #34226
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
addaleax added a commit that referenced this issue Sep 22, 2020
@jasnell @addaleax
doc: document security issues with url.parse()
0ebb30b 
Fixes: #31279

PR-URL: #34226
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@TimothyGu TimothyGu mentioned this issue May 11, 2021
Closed
@kabachook kabachook mentioned this issue Sep 6, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
discuss doc security url
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@sam-github @devsnek