diff --git a/docs/docs/CHANGELOG.md b/docs/docs/CHANGELOG.md index cb77fad2..1b53f06b 100644 --- a/docs/docs/CHANGELOG.md +++ b/docs/docs/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Limit naive honeypot r9k delay to one second. - Fix an obscure case where adding query values to a subrequest match could cause an invalid rule match when using path based matching for protected resources. - Fix an edge case where load average expression values could nil pointer dereference when Anubis just started up. +- Fix an obscure case where Anubis in subrequest mode could allow redirects to invalid domains with strange instructions. - Fix `path_regex` and CEL `path` rules not matching when using Traefik `forwardAuth` middleware. Anubis now checks `X-Forwarded-Uri` (Traefik) in addition to `X-Original-URI` (nginx) when resolving the request path in subrequest mode ([#1628](https://github.com/TecharoHQ/anubis/issues/1628)). ## v1.25.0: Necron diff --git a/lib/http.go b/lib/http.go index 0276282c..96cc1e18 100644 --- a/lib/http.go +++ b/lib/http.go @@ -403,14 +403,15 @@ func (s *Server) ServeHTTPNext(w http.ResponseWriter, r *http.Request) { localizer := localization.GetLocalizer(r) redir := r.FormValue("redir") - urlParsed, err := url.ParseRequestURI(redir) + urlParsed, err := url.Parse(redir) if err != nil { - // if ParseRequestURI fails, try as relative URL - urlParsed, err = r.URL.Parse(redir) - if err != nil { - s.respondWithStatus(w, r, localizer.T("redirect_not_parseable"), makeCode(err), http.StatusBadRequest) - return - } + s.respondWithStatus(w, r, localizer.T("redirect_not_parseable"), makeCode(err), http.StatusBadRequest) + return + } + + if urlParsed.Opaque != "" || (urlParsed.Scheme == "" && strings.HasPrefix(redir, "//")) { + s.respondWithStatus(w, r, localizer.T("invalid_redirect"), "", http.StatusBadRequest) + return } // validate URL scheme to prevent javascript:, data:, file:, tel:, etc. diff --git a/lib/http_test.go b/lib/http_test.go index bacc8358..f1db27aa 100644 --- a/lib/http_test.go +++ b/lib/http_test.go @@ -223,3 +223,17 @@ func TestNoCacheOnError(t *testing.T) { }) } } + +func TestRejectsHostlessRedirect(t *testing.T) { + pol := loadPolicies(t, "testdata/useragent.yaml", 0) + srv := spawnAnubis(t, Options{Policy: pol, RedirectDomains: []string{"allowed.example"}}) + req := httptest.NewRequest(http.MethodGet, "https://anubis.example/.within.website/?redir=%2f%2fevil.example%2fphish", nil) + rr := httptest.NewRecorder() + srv.ServeHTTPNext(rr, req) + if rr.Code != http.StatusBadRequest { + t.Fatalf("expected hostless redirect to be rejected, got HTTP %d body %q", rr.Code, rr.Body.String()) + } + if got := rr.Header().Get("Location"); got != "" { + t.Fatalf("expected no Location header on rejected redirect, got %q", got) + } +}