Skip to content

Commit

Permalink
fix: mitigate allowed redirect domain bypass (#174)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mccutchen committed May 12, 2024
1 parent 8f905de commit 874932b
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
11 changes: 9 additions & 2 deletions httpbin/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,14 @@ func (h *HTTPBin) RedirectTo(w http.ResponseWriter, r *http.Request) {
return
}

if u.IsAbs() && len(h.AllowedRedirectDomains) > 0 {
// If we're given a URL that includes a domain name and we have a list of
// allowed domains, ensure that the domain is allowed.
//
// Note: This checks the hostname directly rather than using the net.URL's
// IsAbs() method, because IsAbs() will return false for URLs that omit
// the scheme but include a domain name, like "//evil.com" and it's
// important that we validate the domain in these cases as well.
if u.Hostname() != "" && len(h.AllowedRedirectDomains) > 0 {
if _, ok := h.AllowedRedirectDomains[u.Hostname()]; !ok {
// for this error message we do not use our standard JSON response
// because we want it to be more obviously human readable.
Expand Down Expand Up @@ -957,7 +964,7 @@ func (h *HTTPBin) doLinksPage(w http.ResponseWriter, _ *http.Request, n int, off
// doRedirect set redirect header
func (h *HTTPBin) doRedirect(w http.ResponseWriter, path string, code int) {
var sb strings.Builder
if strings.HasPrefix(path, "/") {
if strings.HasPrefix(path, "/") && !strings.HasPrefix(path, "//") {
sb.WriteString(h.prefix)
}
sb.WriteString(path)
Expand Down
3 changes: 3 additions & 0 deletions httpbin/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1400,6 +1400,9 @@ func TestRedirectTo(t *testing.T) {
{"/redirect-to?url=https://example.org/foo/bar", http.StatusFound}, // paths don't matter
{"/redirect-to?url=https://foo.example.org/foo/bar", http.StatusForbidden}, // subdomains of allowed domains do not match
{"/redirect-to?url=https://evil.com", http.StatusForbidden}, // not in allowlist

// See https://github.com/mccutchen/go-httpbin/issues/173
{"/redirect-to?url=//evil.com", http.StatusForbidden}, // missing scheme to attempt to bypass allowlist
}
for _, test := range allowListTests {
test := test
Expand Down

0 comments on commit 874932b

Please sign in to comment.