From 2cf7d4da99d72ac7038ce0103462ea7a94f3949a Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Wed, 11 Nov 2026 21:54:13 -0800 Subject: [PATCH] fix(middleware): harden redirect URL construction in v4 Backport of #3220. Closes #3217 for v4. --- middleware/redirect.go | 59 ++++++++++++--- middleware/redirect_test.go | 139 ++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 9 deletions(-) diff --git a/middleware/redirect.go b/middleware/redirect.go index b772ac131..1205921e9 100644 --- a/middleware/redirect.go +++ b/middleware/redirect.go @@ -64,7 +64,7 @@ func HTTPSWWWRedirect() echo.MiddlewareFunc { // See `HTTPSWWWRedirect()`. func HTTPSWWWRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc { return redirect(config, func(scheme, host, uri string) (bool, string) { - if scheme != "https" && !strings.HasPrefix(host, www) { + if scheme != "https" && !hasWWWPrefix(host) { return true, "https://www." + host + uri } return false, "" @@ -84,7 +84,7 @@ func HTTPSNonWWWRedirect() echo.MiddlewareFunc { func HTTPSNonWWWRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc { return redirect(config, func(scheme, host, uri string) (ok bool, url string) { if scheme != "https" { - host = strings.TrimPrefix(host, www) + host = stripWWWPrefix(host) return true, "https://" + host + uri } return false, "" @@ -103,7 +103,7 @@ func WWWRedirect() echo.MiddlewareFunc { // See `WWWRedirect()`. func WWWRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc { return redirect(config, func(scheme, host, uri string) (bool, string) { - if !strings.HasPrefix(host, www) { + if !hasWWWPrefix(host) { return true, scheme + "://www." + host + uri } return false, "" @@ -122,8 +122,8 @@ func NonWWWRedirect() echo.MiddlewareFunc { // See `NonWWWRedirect()`. func NonWWWRedirectWithConfig(config RedirectConfig) echo.MiddlewareFunc { return redirect(config, func(scheme, host, uri string) (bool, string) { - if strings.HasPrefix(host, www) { - return true, scheme + "://" + host[4:] + uri + if hasWWWPrefix(host) { + return true, scheme + "://" + host[len(www):] + uri } return false, "" }) @@ -144,12 +144,53 @@ func redirect(config RedirectConfig, cb redirectLogic) echo.MiddlewareFunc { } req, scheme := c.Request(), c.Scheme() + if req.URL.Path == "*" { + return next(c) + } + switch scheme { + case "ws": + scheme = "http" + case "wss": + scheme = "https" + } host := req.Host - if ok, url := cb(scheme, host, req.RequestURI); ok { - return c.Redirect(config.Code, url) + ok, target := cb(scheme, host, "") + if !ok { + return next(c) } - - return next(c) + if !isValidRedirectHost(host) { + return echo.ErrBadRequest + } + uri := req.RequestURI + if uri == "" || uri[0] != '/' { + uri = req.URL.RequestURI() + } + // The host already anchors this absolute URL, so preserve leading path + // slashes. Only control characters need escaping here. + return c.Redirect(config.Code, target+escapeControlChars(uri)) } } } + +func hasWWWPrefix(host string) bool { + return len(host) >= len(www) && strings.EqualFold(host[:len(www)], www) +} + +func stripWWWPrefix(host string) string { + if hasWWWPrefix(host) { + return host[len(www):] + } + return host +} + +// isValidRedirectHost checks the authority before it is used in a Location. +// Check the stripped host too, as removing a bare www. would leave no hostname. +func isValidRedirectHost(host string) bool { + host = stripWWWPrefix(host) + if host == "" || host[0] == ':' || host[0] == '.' { + return false + } + // net/http validates Host syntax. Reject percent escapes too, since a + // browser could interpret them as authority delimiters in Location. + return !strings.Contains(host, "%") +} diff --git a/middleware/redirect_test.go b/middleware/redirect_test.go index a8407f8a6..1bdedb48d 100644 --- a/middleware/redirect_test.go +++ b/middleware/redirect_test.go @@ -324,3 +324,142 @@ func TestRedirectHTTPSRedirect_forwardedProtoTrust(t *testing.T) { }) } } + +func TestRedirectMalformedHost(t *testing.T) { + for _, tc := range []struct { + host string + hasWWW bool + }{{"", false}, {"www.", true}, {"www.:80", true}, {"WWW.", true}, {":80", false}, {".example.com", false}, {"example.com%2fpath", false}} { + for _, mw := range []struct { + name string + make func() echo.MiddlewareFunc + reject bool + }{{"https", HTTPSRedirect, true}, {"https-www", HTTPSWWWRedirect, !tc.hasWWW}, {"https-non-www", HTTPSNonWWWRedirect, true}, {"www", WWWRedirect, !tc.hasWWW}, {"non-www", NonWWWRedirect, tc.hasWWW}} { + t.Run(mw.name+"/"+tc.host, func(t *testing.T) { + e := echo.New() + e.Pre(mw.make()) + called := false + e.GET("/*", func(c echo.Context) error { called = true; return c.NoContent(http.StatusOK) }) + req := httptest.NewRequest(http.MethodGet, "/attacker.com/login", nil) + req.Host = tc.host + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + want := http.StatusOK + if mw.reject { + want = http.StatusBadRequest + } + assert.Equal(t, want, rec.Code) + assert.Equal(t, !mw.reject, called) + assert.Empty(t, rec.Header().Get(echo.HeaderLocation)) + }) + } + } +} + +func TestRedirectBareWWWHostOverHTTP(t *testing.T) { + e := echo.New() + e.Pre(HTTPSNonWWWRedirect()) + e.GET("/*", func(c echo.Context) error { return c.NoContent(http.StatusOK) }) + server := httptest.NewServer(e) + defer server.Close() + req, err := http.NewRequest(http.MethodGet, server.URL+"/attacker.com/login", nil) + if !assert.NoError(t, err) { + return + } + req.Host = "www." + client := server.Client() + client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } + resp, err := client.Do(req) + if !assert.NoError(t, err) { + return + } + defer resp.Body.Close() + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Empty(t, resp.Header.Get(echo.HeaderLocation)) +} + +func TestRedirectRequestTarget(t *testing.T) { + for _, tc := range []struct{ uri, want string }{ + {"", "https://example.com/files/a%2Fb?x=1"}, + {"http://example.com/files/a%2Fb?x=1", "https://example.com/files/a%2Fb?x=1"}, + {"//attacker.com/x", "https://example.com//attacker.com/x"}, + {"/x\t/y", "https://example.com/x%09/y"}, + } { + t.Run(tc.uri, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "http://example.com/files/a%2Fb?x=1", nil) + req.RequestURI = tc.uri + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + assert.NoError(t, HTTPSRedirect()(func(echo.Context) error { return nil })(c)) + assert.Equal(t, tc.want, rec.Header().Get(echo.HeaderLocation)) + }) + } +} + +func TestRedirectValidAuthority(t *testing.T) { + for _, host := range []string{"example.com", "example.com:8080", "[::1]", "[::1]:8080"} { + t.Run(host, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/docs", nil) + req.Host = host + rec := httptest.NewRecorder() + assert.NoError(t, HTTPSRedirect()(func(echo.Context) error { return nil })(e.NewContext(req, rec))) + assert.Equal(t, "https://"+host+"/docs", rec.Header().Get(echo.HeaderLocation)) + }) + } +} + +func TestRedirectWWWCaseAndWebSocketScheme(t *testing.T) { + for _, tc := range []struct { + name, host, proto, want string + make func() echo.MiddlewareFunc + }{ + {"strip uppercase", "WWW.example.com", "http", "http://example.com/docs", NonWWWRedirect}, + {"keep www", "WWW.example.com", "http", "", HTTPSWWWRedirect}, + {"already www", "WWW.example.com", "https", "", WWWRedirect}, + {"ws", "example.com", "ws", "http://www.example.com/docs", WWWRedirect}, + {"wss", "www.example.com", "wss", "https://example.com/docs", NonWWWRedirect}, + {"already secure", "example.com", "wss", "", HTTPSRedirect}, + } { + t.Run(tc.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/docs", nil) + req.Host = tc.host + req.RemoteAddr = "127.0.0.1:1234" + req.Header.Set(echo.HeaderXForwardedProto, tc.proto) + rec := httptest.NewRecorder() + assert.NoError(t, tc.make()(func(echo.Context) error { return nil })(e.NewContext(req, rec))) + assert.Equal(t, tc.want, rec.Header().Get(echo.HeaderLocation)) + }) + } +} + +func TestRedirectAsteriskTarget(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodOptions, "*", nil) + req.Host = "example.com" + rec := httptest.NewRecorder() + called := false + err := HTTPSRedirect()(func(c echo.Context) error { called = true; return c.NoContent(http.StatusNoContent) })(e.NewContext(req, rec)) + assert.NoError(t, err) + assert.True(t, called) + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.Empty(t, rec.Header().Get(echo.HeaderLocation)) +} + +func TestRedirectUnchangedHost(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "https://example.com/docs", nil) + req.Host = "" + rec := httptest.NewRecorder() + called := false + err := HTTPSRedirect()(func(c echo.Context) error { + called = true + return c.NoContent(http.StatusOK) + })(e.NewContext(req, rec)) + assert.NoError(t, err) + assert.True(t, called) + assert.Equal(t, http.StatusOK, rec.Code) + assert.Empty(t, rec.Header().Get(echo.HeaderLocation)) +}