diff --git a/httpbin/handlers.go b/httpbin/handlers.go index f965a6c..8fd3f1f 100644 --- a/httpbin/handlers.go +++ b/httpbin/handlers.go @@ -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. @@ -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) diff --git a/httpbin/handlers_test.go b/httpbin/handlers_test.go index 635ed8a..d9a345b 100644 --- a/httpbin/handlers_test.go +++ b/httpbin/handlers_test.go @@ -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