fix(static): reject encoded path separator to prevent route-level auth bypass

The router matches routes against the raw, still-encoded request path, so an
encoded slash (%2F) is not treated as a segment separator -- /admin%2Fsecret.txt
is a single segment and never matches a protected /admin/* group. The request
then falls through to the static handler, which unescaped %2F back to "/" before
resolving the file, so admin/secret.txt was served from disk while the group's
auth middleware never ran. This bypasses route-level access control and discloses
static files (GHSA-vfp3-v2gw-7wfq, CWE-22).

Reject a static wildcard containing an encoded path separator (%2F/%2f or
%5C/%5c) with 404 before unescaping, keeping the router and the file handler
consistent about what is a separator. No real filename contains a separator, so
legitimate requests are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: vishr

echo.go

@@ -589,6 +589,14 @@ func StaticDirectoryHandler(fileSystem fs.FS, disablePathUnescaping bool) Handle
 	return func(c *Context) error {
 		p := c.Param("*")
 		if !disablePathUnescaping { // when router is already unescaping we do not want to do is twice
+			// The router matches routes against the raw, still-encoded request path, so an
+			// encoded path separator (%2F or %5C) is not treated as a segment boundary during
+			// routing. Unescaping it here would let it act as a separator and resolve a file
+			// outside the path the router authorized, bypassing route-level middleware (e.g. auth
+			// on a sibling route). No real filename contains a separator, so reject it as not found.
+			if hasEncodedPathSeparator(p) {
+				return ErrNotFound
+			}
 			tmpPath, err := url.PathUnescape(p)
 			if err != nil {
 				return fmt.Errorf("failed to unescape path variable: %w", err)
@@ -613,6 +621,25 @@ func StaticDirectoryHandler(fileSystem fs.FS, disablePathUnescaping bool) Handle
 	}
 }
 
+// hasEncodedPathSeparator reports whether s contains a percent-encoded path
+// separator: %2F/%2f (slash) or %5C/%5c (backslash). Such sequences let an
+// attacker smuggle a separator past the router, which matches on the raw encoded
+// path, so they must be rejected before unescaping when resolving static files.
+func hasEncodedPathSeparator(s string) bool {
+	for i := 0; i+2 < len(s); i++ {
+		if s[i] != '%' {
+			continue
+		}
+		switch {
+		case s[i+1] == '2' && (s[i+2] == 'f' || s[i+2] == 'F'): // %2F
+			return true
+		case s[i+1] == '5' && (s[i+2] == 'c' || s[i+2] == 'C'): // %5C
+			return true
+		}
+	}
+	return false
+}
+
 // FileFS registers a new route with path to serve file from the provided file system.
 //
 // Avoid using the leading `/` slash as most of the Go standard library fs.FS implementations require relative paths for

echo_test.go

@@ -259,13 +259,16 @@ func TestEcho_StaticFS(t *testing.T) {
 			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
 		},
 		{
-			name:                 "open redirect vulnerability",
+			// An encoded slash (%2f) is rejected outright (GHSA-vfp3-v2gw-7wfq): the router
+			// matches on the raw path so %2f is not a separator, and unescaping it here would
+			// let it act as one. No redirect is emitted, closing the open-redirect vector.
+			name:                 "encoded slash is rejected, not redirected",
 			givenPrefix:          "/",
 			givenFs:              os.DirFS("_fixture/"),
 			whenURL:              "/open.redirect.hackercom%2f..",
-			expectStatus:         http.StatusMovedPermanently,
-			expectHeaderLocation: "/open.redirect.hackercom/../", // location starting with `//open` would be very bad
-			expectBodyStartsWith: "",
+			expectStatus:         http.StatusNotFound,
+			expectHeaderLocation: "",
+			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
 		},
 	}
 

static_encoded_separator_test.go

@@ -0,0 +1,63 @@
+package echo
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"testing/fstest"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// Regression for GHSA-vfp3-v2gw-7wfq: an encoded slash (%2F) must not let a static
+// file request resolve across a path separator and bypass route-level middleware.
+func TestStaticDirectoryHandler_EncodedSeparatorDoesNotBypassRoute(t *testing.T) {
+	fsys := fstest.MapFS{
+		"admin/secret.txt": {Data: []byte("TOP-SECRET")},
+		"index.html":       {Data: []byte("public")},
+	}
+	e := New()
+	g := e.Group("/admin", func(next HandlerFunc) HandlerFunc {
+		return func(c *Context) error { return c.String(http.StatusForbidden, "denied") }
+	})
+	g.GET("/*", func(c *Context) error { return c.String(http.StatusOK, "reached-protected-handler") })
+	e.StaticFS("/", fsys)
+
+	cases := []struct {
+		target   string
+		wantCode int
+		wantBody string
+	}{
+		{"/admin/secret.txt", http.StatusForbidden, "denied"}, // protected route fires
+		{"/admin%2Fsecret.txt", http.StatusNotFound, ""},      // encoded slash rejected, no disclosure
+		{"/admin%2fsecret.txt", http.StatusNotFound, ""},      // lower-case hex variant
+		{"/index.html", http.StatusOK, "public"},              // legitimate static file still served
+	}
+	for _, tc := range cases {
+		req := httptest.NewRequest(http.MethodGet, tc.target, nil)
+		rec := httptest.NewRecorder()
+		e.ServeHTTP(rec, req)
+		assert.Equal(t, tc.wantCode, rec.Code, "GET %s", tc.target)
+		if tc.wantBody != "" {
+			assert.Equal(t, tc.wantBody, rec.Body.String(), "GET %s", tc.target)
+		}
+		assert.NotContains(t, rec.Body.String(), "TOP-SECRET", "GET %s leaked protected file", tc.target)
+	}
+}
+
+func TestHasEncodedPathSeparator(t *testing.T) {
+	for s, want := range map[string]bool{
+		"foo/bar.txt": false,
+		"100%25.txt":  false, // encoded percent, not a separator
+		"a%2Fb":       true,
+		"a%2fb":       true,
+		"a%5Cb":       true,
+		"a%5cb":       true,
+		"trailing%2F": true,
+		"%2F":         true,
+		"%2":          false, // truncated, not a full sequence
+		"":            false,
+	} {
+		assert.Equal(t, want, hasEncodedPathSeparator(s), "input=%q", s)
+	}
+}