fix(middleware): prevent open redirect (CWE-601) in Redirect middlewares

middleware/redirect.go

@@ -134,8 +134,22 @@ func (config RedirectConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
 			}
 
 			req, scheme := c.Request(), c.Scheme()
+			switch scheme {
+			case "ws":
+				scheme = "http"
+			case "wss":
+				scheme = "https"
+			}
 			host := req.Host
-			if ok, url := config.redirect(scheme, host, req.RequestURI); ok {
+			if !isValidRedirectHost(host) {
+				return next(c)
+			}
+			uri := req.RequestURI
+			if uri == "" || uri[0] != '/' {
+				uri = req.URL.RequestURI()
+			}
+			uri = sanitizeURI(uri)
+			if ok, url := config.redirect(scheme, host, uri); ok {
 				return c.Redirect(config.Code, url)
 			}
 
@@ -144,6 +158,31 @@ func (config RedirectConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
 	}, nil
 }
 
+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
+}
+
+func isValidRedirectHost(host string) bool {
+	host = stripWWWPrefix(host)
+	if host == "" || host[0] == ':' || host[0] == '.' {
+		return false
+	}
+	for i := 0; i < len(host); i++ {
+		ch := host[i]
+		if ch <= 0x20 || ch >= 0x7f || strings.IndexByte(`/\@?#`, ch) != -1 {
+			return false
+		}
+	}
+	return true
+}
+
 var redirectHTTPS = func(scheme, host, uri string) (bool, string) {
 	if scheme != "https" {
 		return true, "https://" + host + uri
@@ -153,32 +192,32 @@ var redirectHTTPS = func(scheme, host, uri string) (bool, string) {
 
 var redirectHTTPSWWW = func(scheme, host, uri string) (bool, string) {
 	// Redirect if not HTTPS OR missing www prefix (needs either fix)
-	if scheme != "https" || !strings.HasPrefix(host, www) {
-		host = strings.TrimPrefix(host, www) // Remove www if present to avoid duplication
+	if scheme != "https" || !hasWWWPrefix(host) {
+		host = stripWWWPrefix(host) // Remove www if present to avoid duplication
 		return true, "https://www." + host + uri
 	}
 	return false, ""
 }
 
 var redirectNonHTTPSWWW = func(scheme, host, uri string) (ok bool, url string) {
 	// Redirect if not HTTPS OR has www prefix (needs either fix)
-	if scheme != "https" || strings.HasPrefix(host, www) {
-		host = strings.TrimPrefix(host, www)
+	if scheme != "https" || hasWWWPrefix(host) {
+		host = stripWWWPrefix(host)
 		return true, "https://" + host + uri
 	}
 	return false, ""
 }
 
 var redirectWWW = func(scheme, host, uri string) (bool, string) {
-	if !strings.HasPrefix(host, www) {
+	if !hasWWWPrefix(host) {
 		return true, scheme + "://www." + host + uri
 	}
 	return false, ""
 }
 
 var redirectNonWWW = 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, ""
 }

middleware/redirect_test.go

@@ -336,3 +336,114 @@ func TestRedirectHTTPSRedirect_forwardedProtoTrust(t *testing.T) {
 		})
 	}
 }
+
+func TestRedirectSecurityHardening(t *testing.T) {
+	var testCases = []struct {
+		name             string
+		middleware       middlewareGenerator
+		whenHost         string
+		whenRequestURI   string
+		whenHeader       http.Header
+		expectLocation   string
+		expectStatusCode int
+	}{
+		{
+			name:             "NonWWWRedirect ignores bare www. host to prevent https:///attacker.com open redirect",
+			middleware:       NonWWWRedirect,
+			whenHost:         "www.",
+			whenRequestURI:   "/attacker.com/login",
+			expectLocation:   "",
+			expectStatusCode: http.StatusOK,
+		},
+		{
+			name:             "HTTPSNonWWWRedirect ignores bare www. host to prevent https:///attacker.com open redirect",
+			middleware:       HTTPSNonWWWRedirect,
+			whenHost:         "www.",
+			whenRequestURI:   "/attacker.com/login",
+			expectLocation:   "",
+			expectStatusCode: http.StatusOK,
+		},
+		{
+			name:             "HTTPSRedirect ignores empty host",
+			middleware:       HTTPSRedirect,
+			whenHost:         "",
+			whenRequestURI:   "/attacker.com",
+			expectLocation:   "",
+			expectStatusCode: http.StatusOK,
+		},
+		{
+			name:             "HTTPSRedirect ignores host with authority or path delimiters",
+			middleware:       HTTPSRedirect,
+			whenHost:         "attacker.com@labstack.com",
+			whenRequestURI:   "/",
+			expectLocation:   "",
+			expectStatusCode: http.StatusOK,
+		},
+		{
+			name:             "NonWWWRedirect handles uppercase WWW. prefix",
+			middleware:       NonWWWRedirect,
+			whenHost:         "WWW.labstack.com",
+			whenRequestURI:   "/docs",
+			expectLocation:   "http://labstack.com/docs",
+			expectStatusCode: http.StatusMovedPermanently,
+		},
+		{
+			name:             "HTTPSWWWRedirect does not duplicate uppercase WWW. prefix",
+			middleware:       HTTPSWWWRedirect,
+			whenHost:         "WWW.labstack.com",
+			whenRequestURI:   "/docs",
+			expectLocation:   "https://www.labstack.com/docs",
+			expectStatusCode: http.StatusMovedPermanently,
+		},
+		{
+			name:             "HTTPSRedirect sanitizes double-slash and control chars in RequestURI",
+			middleware:       HTTPSRedirect,
+			whenHost:         "labstack.com",
+			whenRequestURI:   "//attacker.com/phish",
+			expectLocation:   "https://labstack.com/attacker.com/phish",
+			expectStatusCode: http.StatusMovedPermanently,
+		},
+		{
+			name:             "WWWRedirect normalizes ws scheme from X-Forwarded-Proto to http",
+			middleware:       WWWRedirect,
+			whenHost:         "labstack.com",
+			whenRequestURI:   "/ws",
+			whenHeader:       http.Header{echo.HeaderXForwardedProto: {"ws"}},
+			expectLocation:   "http://www.labstack.com/ws",
+			expectStatusCode: http.StatusMovedPermanently,
+		},
+		{
+			name:             "HTTPSRedirect treats wss scheme from X-Forwarded-Proto as https",
+			middleware:       HTTPSRedirect,
+			whenHost:         "labstack.com",
+			whenRequestURI:   "/ws",
+			whenHeader:       http.Header{echo.HeaderXForwardedProto: {"wss"}},
+			expectLocation:   "",
+			expectStatusCode: http.StatusOK,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := echo.New()
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			req.Host = tc.whenHost
+			req.RequestURI = tc.whenRequestURI
+			req.RemoteAddr = "10.0.0.1:1234"
+			if tc.whenHeader != nil {
+				req.Header = tc.whenHeader
+			}
+			res := httptest.NewRecorder()
+			c := e.NewContext(req, res)
+
+			err := tc.middleware()(func(c *echo.Context) error {
+				return c.NoContent(http.StatusOK)
+			})(c)
+
+			assert.NoError(t, err)
+			assert.Equal(t, tc.expectStatusCode, res.Code)
+			assert.Equal(t, tc.expectLocation, res.Header().Get(echo.HeaderLocation))
+		})
+	}
+}
+