middleware: harden RedirectSlashes handler (#1044)

pkieltyka · web-flow · commit 6eb35881c0e4 · 2026-01-14T11:02:00.000-05:00
diff --git a/middleware/strip.go b/middleware/strip.go
@@ -47,15 +47,22 @@ func RedirectSlashes(next http.Handler) http.Handler {
 		} else {
 			path = r.URL.Path
 		}
+
 		if len(path) > 1 && path[len(path)-1] == '/' {
-			// Trim all leading and trailing slashes (e.g., "//evil.com", "/some/path//")
-			path = "/" + strings.Trim(path, "/")
+			// Normalize backslashes to forward slashes to prevent "/\evil.com" style redirects
+			// that some clients may interpret as protocol-relative.
+			path = strings.ReplaceAll(path, `\`, `/`)
+
+			// Collapse leading/trailing slashes and force a single leading slash.
+			path := "/" + strings.Trim(path, "/")
+
 			if r.URL.RawQuery != "" {
 				path = fmt.Sprintf("%s?%s", path, r.URL.RawQuery)
 			}
 			http.Redirect(w, r, path, 301)
 			return
 		}
+
 		next.ServeHTTP(w, r)
 	}
 	return http.HandlerFunc(fn)
diff --git a/middleware/strip_test.go b/middleware/strip_test.go
@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/go-chi/chi/v5"
@@ -271,3 +272,58 @@ func TestStripPrefix(t *testing.T) {
 		t.Fatalf("got: %q, want: %q", resp, "404 page not found\n")
 	}
 }
+
+func TestRedirectSlashes_PreventBackslashRelativeOpenRedirect(t *testing.T) {
+	h := RedirectSlashes(http.NotFoundHandler())
+
+	tests := []struct {
+		name   string
+		target string
+	}{
+		{
+			name:   `raw backslash: /\evil.com/`,
+			target: `/\evil.com/`,
+		},
+		{
+			name:   `encoded backslash: /%5Cevil.com/`,
+			target: "/%5Cevil.com/",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, "http://example.test"+tc.target, nil)
+			rr := httptest.NewRecorder()
+
+			h.ServeHTTP(rr, req)
+			res := rr.Result()
+			defer res.Body.Close()
+
+			if res.StatusCode != http.StatusMovedPermanently {
+				t.Fatalf("expected %d, got %d", http.StatusMovedPermanently, res.StatusCode)
+			}
+
+			loc := res.Header.Get("Location")
+			if loc == "" {
+				t.Fatalf("expected Location header to be set")
+			}
+
+			// The core security assertions:
+			if strings.Contains(loc, `\`) {
+				t.Fatalf("Location must not contain backslashes: %q", loc)
+			}
+			if strings.HasPrefix(loc, "//") {
+				t.Fatalf("Location must not be protocol-relative: %q", loc)
+			}
+			if !strings.HasPrefix(loc, "/") {
+				t.Fatalf("Location must be an absolute-path reference starting with '/': %q", loc)
+			}
+
+			// Optional stronger assertion if your middleware normalizes to /evil.com exactly:
+			// (Keep or remove depending on your chosen behavior.)
+			if loc != "/evil.com" {
+				t.Fatalf("expected Location %q, got %q", "/evil.com", loc)
+			}
+		})
+	}
+}
