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

Strengthen check for absolute redirect #173

Closed
mccutchen opened this issue Apr 23, 2024 · 2 comments · Fixed by #174
Closed

Strengthen check for absolute redirect #173

mccutchen opened this issue Apr 23, 2024 · 2 comments · Fixed by #174

Comments

@mccutchen
Copy link
Owner

mccutchen commented Apr 23, 2024

We got a security alert regarding this line of code in the doRedirect() helper function:

if strings.HasPrefix(path, "/") {

Redirect URLs should be checked to ensure that user input cannot cause a site to redirect to arbitrary domains. This is often done with a check that the redirect URL begins with a slash, which most of the time is an absolute redirect on the same host. However, browsers interpret URLs beginning with // or /\ as absolute URLs. For example, a redirect to //example.com will redirect to https://example.com. Thus, redirect checks must also check the second character of redirect URLs.

@sarathsp06
Copy link

@mccutchen unable to access the link above

@mccutchen
Copy link
Owner Author

@sarathsp06 ah, my bad, I guess it makes some sense that the code scanning alerts would be private. I've copied the relevant context to the issue itself.

mccutchen added a commit that referenced this issue May 12, 2024
Before this change, it was possible to bypass go-httpbin's allowed
redirect domain configuration by passing an absolute URL without a
scheme (e.g. `//evil.com`) to the `/redirect-to` endpoint.

Fixes #173.
mccutchen added a commit that referenced this issue May 12, 2024
Before this change, it was possible to bypass go-httpbin's allowed
redirect domain configuration by passing an absolute URL without a
scheme (e.g. `//evil.com`) to the `/redirect-to` endpoint.

Fixes #173.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants