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

net/http: Redirect hardening #71161

Open
neild opened this issue Jan 7, 2025 · 5 comments
Open

net/http: Redirect hardening #71161

neild opened this issue Jan 7, 2025 · 5 comments
Assignees
Labels
LibraryProposal Issues describing a requested change to the Go standard library or x/ libraries, but not to a tool NeedsFix The path to resolution is known, but the work has not been done. Security

Comments

@neild
Copy link
Contributor

neild commented Jan 7, 2025

The http.Redirect function writes a 3xx redirect to a ResponseWriter.

Redirect takes a URL parameter, of type string. The URL parameter has only minimal sanitization applied, and is not safe for use with attacker-controlled inputs.

One example of possibly-surprising behavior is that a redirect to \\example.com is a relative-path reference according to RFC 3986, but will be interpreted by most browsers as a network-path reference. /\example.com is an absolute-path reference according to the RFC, but will also be interpreted by browsers as a network-path reference. (Thanks to Jingcheng Yang (Sichuan University), Enze Wang@IPASSLAB(@zer0yu), Jianjun Chen (Tsinghua University & Zhongguancun Laboratory) for reporting this case.)

We should document that Redirect does not sanitize its URL parameter. Users who wish to use Redirect with untrusted URLs should parse the URL with net/url, perform whatever validation they may wish, and then reassemble the parsed and validated URL into a string with net/url.URL.String.

We should also consider, as a hardening measure, %-encoding backslashes at the start of Redirect's URL parameter to prevent browsers from interpreting them as part of an absolute-path reference.

@neild neild self-assigned this Jan 7, 2025
@gabyhelp
Copy link

gabyhelp commented Jan 7, 2025

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/640918 mentions this issue: net/http: escape leading \s in Redirect targets

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/640917 mentions this issue: net/http: document that Redirect does not sanitize its url

@zer0yu
Copy link

zer0yu commented Jan 9, 2025

Thank you for addressing this issue and for implementing the fix!

@seankhliao seankhliao added Security NeedsFix The path to resolution is known, but the work has not been done. labels Jan 9, 2025
@gabyhelp gabyhelp added the LibraryProposal Issues describing a requested change to the Go standard library or x/ libraries, but not to a tool label Jan 10, 2025
@P3ngu1nW
Copy link

Thank you for addressing this issue and implementing the proposed fixes.
Since this issue has been tagged as Security, I would like to inquire if it would be possible to assign a CVE (Common Vulnerabilities and Exposures) ID to this vulnerability for tracking purposes. A CVE would help ensure the issue is properly documented and referenced in security advisories.
Looking forward to your thoughts on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LibraryProposal Issues describing a requested change to the Go standard library or x/ libraries, but not to a tool NeedsFix The path to resolution is known, but the work has not been done. Security
Projects
None yet
Development

No branches or pull requests

6 participants