Skip to content

Commit 91f630e

Browse files
vishrclaude
andcommitted
fix(static): disable static path unescaping by default to prevent ACL bypass
Fixes GHSA-3pmx-cf9f-34xr, a bypass of the GHSA-vfp3-v2gw-7wfq fix. The router matches the raw, still-encoded request path, so encoded separators and dot segments in a static wildcard are not seen as traversal during routing. Unescaping them in the static file resolver afterwards let an attacker reach a file across a route-level middleware guard the encoded path never matched: - /public/%2E%2E/admin/secret.txt resolved to admin/secret.txt (high severity, default router) - /public%2F..%2Fadmin%2Fsecret.txt under UseEscapedPathForMatching=true, where the router decodes the path itself before the handler sees it Rather than keep extending an encoding denylist, address the root cause: make static path unescaping opt-in. - echo: Config.EnablePathUnescapingStaticFiles (default false) controls unescaping for Echo.Static/StaticFS and Group.Static/StaticFS. - middleware: StaticConfig.EnablePathUnescaping replaces the now deprecated DisablePathUnescaping (default is the safe, no-unescape mode). With unescaping off, %2F/%5C/%2E%2E stay literal and never become separators or traversal. As defense in depth, and to also close the UseEscapedPathForMatching variant (where the router, not the handler, does the decoding), reject any ".." path segment in the resolved wildcard via pathutil.HasDotDotSegment, mirroring the fs.ValidPath "no .. element" invariant. The existing encoded-separator guard remains as a backstop on the opt-in unescaping path. BREAKING CHANGE: static files whose names contain URL-encoded characters (e.g. "hello world.txt" via /hello%20world.txt) are no longer served by default; set EnablePathUnescapingStaticFiles / EnablePathUnescaping to opt back in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5786024 commit 91f630e

7 files changed

Lines changed: 276 additions & 10 deletions

File tree

echo.go

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ type Echo struct {
111111

112112
// formParseMaxMemory is passed to Context for multipart form parsing (See http.Request.ParseMultipartForm)
113113
formParseMaxMemory int64
114+
115+
// enablePathUnescapingStaticFiles unescapes the static wildcard param (set via
116+
// Config.EnablePathUnescapingStaticFiles). Default false (safe): see that field.
117+
enablePathUnescapingStaticFiles bool
114118
}
115119

116120
// JSONSerializer is the interface that encodes and decodes JSON to and from interfaces.
@@ -299,6 +303,21 @@ type Config struct {
299303
// FormParseMaxMemory is default value for memory limit that is used
300304
// when parsing multipart forms (See (*http.Request).ParseMultipartForm)
301305
FormParseMaxMemory int64
306+
307+
// EnablePathUnescapingStaticFiles enables unescaping of the wildcard param (param: *)
308+
// for static file methods (Echo.Static, Echo.StaticFS, Group.Static, Group.StaticFS).
309+
//
310+
// Default false (safe): the router matches the raw, still-encoded request path, so
311+
// encoded separators/dot segments (%2F, %5C, %2E%2E) are NOT decoded when resolving
312+
// the file. This prevents ACL bypass where e.g. /admin%2Fprivate.txt or
313+
// /public/%2E%2E/admin/secret.txt resolve a file across a route-level middleware guard
314+
// the encoded path never matched (GHSA-vfp3-v2gw-7wfq, GHSA-3pmx-cf9f-34xr).
315+
//
316+
// Set to true only when serving files whose names contain URL-encoded characters
317+
// (e.g. "hello world.txt" via /hello%20world.txt) and you do not rely on route-based
318+
// ACL guards to restrict access. Encoded separators and ".." segments are still
319+
// rejected even when enabled.
320+
EnablePathUnescapingStaticFiles bool
302321
}
303322

304323
// NewWithConfig creates an instance of Echo with given configuration.
@@ -337,6 +356,7 @@ func NewWithConfig(config Config) *Echo {
337356
if config.FormParseMaxMemory > 0 {
338357
e.formParseMaxMemory = config.FormParseMaxMemory
339358
}
359+
e.enablePathUnescapingStaticFiles = config.EnablePathUnescapingStaticFiles
340360
return e
341361
}
342362

@@ -567,7 +587,7 @@ func (e *Echo) Static(pathPrefix, fsRoot string, middleware ...MiddlewareFunc) R
567587
return e.Add(
568588
http.MethodGet,
569589
pathPrefix+"*",
570-
StaticDirectoryHandler(subFs, false),
590+
StaticDirectoryHandler(subFs, !e.enablePathUnescapingStaticFiles),
571591
middleware...,
572592
)
573593
}
@@ -581,7 +601,7 @@ func (e *Echo) StaticFS(pathPrefix string, filesystem fs.FS, middleware ...Middl
581601
return e.Add(
582602
http.MethodGet,
583603
pathPrefix+"*",
584-
StaticDirectoryHandler(filesystem, false),
604+
StaticDirectoryHandler(filesystem, !e.enablePathUnescapingStaticFiles),
585605
middleware...,
586606
)
587607
}
@@ -609,6 +629,17 @@ func StaticDirectoryHandler(fileSystem fs.FS, disablePathUnescaping bool) Handle
609629
p = tmpPath
610630
}
611631

632+
// Reject parent-directory traversal in the resolved wildcard. A ".." segment is
633+
// only ever present here because it was decoded after routing (encoded "%2E%2E"
634+
// when unescaping is enabled, or decoded by the router itself when
635+
// UseEscapedPathForMatching is set); the router matched a prefix that never
636+
// contained it. Resolving it would cross the route/static-mount boundary the
637+
// match authorized, bypassing route-level middleware (GHSA-3pmx-cf9f-34xr).
638+
if pathutil.HasDotDotSegment(p) {
639+
return NewHTTPError(http.StatusNotFound, http.StatusText(http.StatusNotFound)).
640+
Wrap(fmt.Errorf("rejected dot-dot path segment in static path %q", p))
641+
}
642+
612643
// fs.FS.Open() already assumes that file names are relative to FS root path and considers name with prefix `/` as invalid.
613644
// Use path.Clean (not filepath.Clean): fs.FS paths are always forward-slash, so a backslash must stay a literal
614645
// character rather than being interpreted as a separator on Windows (which would resolve a file across a boundary

group.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func (g *Group) StaticFS(pathPrefix string, filesystem fs.FS, middleware ...Midd
123123
return g.Add(
124124
http.MethodGet,
125125
pathPrefix+"*",
126-
StaticDirectoryHandler(filesystem, false),
126+
StaticDirectoryHandler(filesystem, !g.echo.enablePathUnescapingStaticFiles),
127127
middleware...,
128128
)
129129
}

internal/pathutil/pathutil.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
// middleware package without becoming part of the public API.
77
package pathutil
88

9+
import "strings"
10+
911
// HasEncodedPathSeparator reports whether s contains a percent-encoded path
1012
// separator, case-insensitively: %2F/%2f (forward slash) or %5C/%5c (backslash).
1113
// Backslash is included as defense-in-depth against Windows-style separators even
@@ -28,3 +30,33 @@ func HasEncodedPathSeparator(s string) bool {
2830
}
2931
return false
3032
}
33+
34+
// HasDotDotSegment reports whether p, split on '/', contains a ".." path segment.
35+
// Both ends of the string and every '/' act as boundaries, so "..", "../x", "x/..",
36+
// and "a/../b" match while "..foo" and "foo.." do not.
37+
//
38+
// A ".." segment is parent-directory traversal. The router matches a specific path
39+
// prefix, but a ".." that only becomes a real segment after decoding (encoded as
40+
// "%2E%2E", or decoded by the router itself when UseEscapedPathForMatching is set)
41+
// is never seen as traversal during routing. Cleaning it then resolves a file
42+
// across the route or static-mount boundary the matched route authorized, bypassing
43+
// route-level middleware (GHSA-3pmx-cf9f-34xr). This mirrors the "no .. element"
44+
// invariant of fs.ValidPath; no real filename is "..", so reject it.
45+
//
46+
// Only '/' is a separator here: encoded backslashes are rejected earlier by
47+
// HasEncodedPathSeparator, and on a forward-slash fs.FS a literal backslash never
48+
// acts as a separator, so a "..\\" segment cannot traverse.
49+
func HasDotDotSegment(p string) bool {
50+
for len(p) > 0 {
51+
seg := p
52+
if i := strings.IndexByte(p, '/'); i >= 0 {
53+
seg, p = p[:i], p[i+1:]
54+
} else {
55+
p = ""
56+
}
57+
if seg == ".." {
58+
return true
59+
}
60+
}
61+
return false
62+
}

internal/pathutil/pathutil_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,23 @@ func TestHasEncodedPathSeparator(t *testing.T) {
2525
assert.Equal(t, want, HasEncodedPathSeparator(s), "input=%q", s)
2626
}
2727
}
28+
29+
func TestHasDotDotSegment(t *testing.T) {
30+
for s, want := range map[string]bool{
31+
"..": true,
32+
"../x": true,
33+
"x/..": true,
34+
"a/../b": true,
35+
"public/../admin/sec.txt": true,
36+
"/..": true, // leading slash
37+
"foo/bar.txt": false,
38+
"..foo": false, // not a standalone segment
39+
"foo..": false,
40+
"...": false, // three dots is a real name
41+
"%2E%2E": false, // still-encoded, not yet a separator-bounded ".."
42+
`a/..\b`: false, // backslash is literal on fs.FS, segment is "..\b"
43+
"": false,
44+
} {
45+
assert.Equal(t, want, HasDotDotSegment(s), "input=%q", s)
46+
}
47+
}

middleware/static.go

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,25 @@ type StaticConfig struct {
5454
// Optional. Default value false.
5555
IgnoreBase bool
5656

57-
// DisablePathUnescaping disables path parameter (param: *) unescaping. This is useful when router is set to unescape
58-
// all parameter and doing it again in this middleware would corrupt filename that is requested.
57+
// Deprecated: use EnablePathUnescaping instead. With unescaping now disabled by
58+
// default, this flag is redundant; when set it still forces unescaping off (the
59+
// safe direction) so existing code keeps working. Will be removed in a future version.
5960
DisablePathUnescaping bool
6061

62+
// EnablePathUnescaping enables path parameter (param: *) unescaping.
63+
//
64+
// Default false (safe): the router matches the raw, still-encoded request path, so
65+
// encoded separators/dot segments (%2F, %5C, %2E%2E) are NOT decoded when resolving
66+
// the file. This prevents ACL bypass where e.g. /admin%2Fprivate.txt or
67+
// /public/%2E%2E/admin/secret.txt resolve a file across a route-level middleware guard
68+
// the encoded path never matched (GHSA-vfp3-v2gw-7wfq, GHSA-3pmx-cf9f-34xr).
69+
//
70+
// Set to true only when serving files whose names contain URL-encoded characters
71+
// (e.g. "hello world.txt" via /hello%20world.txt) and you do not rely on route-based
72+
// ACL guards to restrict access. Encoded separators and ".." segments are still
73+
// rejected even when enabled.
74+
EnablePathUnescaping bool
75+
6176
// DirectoryListTemplate is template to list directory contents
6277
// Optional. Default to `directoryListHTMLTemplate` constant below.
6378
DirectoryListTemplate string
@@ -200,14 +215,14 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
200215
// URL.Path is already decoded by net/http, so it must not be unescaped
201216
// again (doing so breaks file names containing '%', see #2599). Only the
202217
// wildcard param from a group route (set below) may still be escaped.
203-
pathUnescape := false
204218
if strings.HasSuffix(c.Path(), "*") { // When serving from a group, e.g. `/static*`.
205219
p = c.Param("*")
206-
pathUnescape = !config.DisablePathUnescaping // because router could already do PathUnescape
207220
}
208-
if pathUnescape {
209-
// The router matched on the raw, still-encoded path (by default), so an encoded
210-
// path separator in the wildcard would only now become a real separator and
221+
// Unescaping is opt-in (see EnablePathUnescaping). DisablePathUnescaping is the
222+
// deprecated inverse and, if set, still forces it off (the safe direction).
223+
if config.EnablePathUnescaping && !config.DisablePathUnescaping {
224+
// The router matched on the raw, still-encoded path, so an encoded path
225+
// separator in the wildcard would only now become a real separator and
211226
// resolve a file the matched route never authorized, bypassing route-level
212227
// middleware. Reject it before unescaping (see echo.StaticDirectoryHandler).
213228
if pathutil.HasEncodedPathSeparator(p) {
@@ -219,6 +234,15 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
219234
return err
220235
}
221236
}
237+
// Reject parent-directory traversal: a ".." segment present here was decoded
238+
// after routing (encoded "%2E%2E", or decoded by the router itself when
239+
// UseEscapedPathForMatching is set), so the matched route never saw it as
240+
// traversal. Resolving it would cross the route/static-mount boundary the match
241+
// authorized, bypassing route-level middleware (GHSA-3pmx-cf9f-34xr).
242+
if pathutil.HasDotDotSegment(p) {
243+
return echo.NewHTTPError(http.StatusNotFound, http.StatusText(http.StatusNotFound)).
244+
Wrap(fmt.Errorf("rejected dot-dot path segment in static path %q", p))
245+
}
222246
// Security: We use path.Clean() (not filepath.Clean()) because:
223247
// 1. HTTP URLs always use forward slashes, regardless of server OS
224248
// 2. path.Clean() provides platform-independent behavior for URL paths

middleware/static_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,73 @@ func TestStatic_EncodedSeparatorDoesNotBypassRoute(t *testing.T) {
309309
}
310310
}
311311

312+
// Regression for GHSA-3pmx-cf9f-34xr in the static middleware: encoded or
313+
// router-decoded ".." must not resolve a file across a route-level guard.
314+
func TestStatic_EncodedDotDotDoesNotBypassRoute(t *testing.T) {
315+
run := func(t *testing.T, e *echo.Echo) {
316+
fsys := fstest.MapFS{
317+
"admin/secret.txt": {Data: []byte("TOP-SECRET")},
318+
"public/ok.txt": {Data: []byte("public")},
319+
}
320+
g := e.Group("/files", StaticWithConfig(StaticConfig{Filesystem: fsys}))
321+
g.GET("/*", func(c *echo.Context) error { return echo.ErrNotFound })
322+
323+
cases := []struct {
324+
target string
325+
wantCode int
326+
}{
327+
{"/files/public/ok.txt", http.StatusOK},
328+
{"/files/public/%2E%2E/admin/secret.txt", http.StatusNotFound},
329+
{"/files/public%2F..%2Fadmin%2Fsecret.txt", http.StatusNotFound},
330+
{"/files/public/../admin/secret.txt", http.StatusNotFound},
331+
}
332+
for _, tc := range cases {
333+
req := httptest.NewRequest(http.MethodGet, tc.target, nil)
334+
rec := httptest.NewRecorder()
335+
e.ServeHTTP(rec, req)
336+
assert.Equal(t, tc.wantCode, rec.Code, "GET %s", tc.target)
337+
assert.NotContains(t, rec.Body.String(), "TOP-SECRET", "GET %s leaked protected file", tc.target)
338+
}
339+
}
340+
341+
t.Run("default router", func(t *testing.T) { run(t, echo.New()) })
342+
t.Run("UseEscapedPathForMatching", func(t *testing.T) {
343+
run(t, echo.NewWithConfig(echo.Config{Router: echo.NewRouter(echo.RouterConfig{UseEscapedPathForMatching: true})}))
344+
})
345+
}
346+
347+
// With EnablePathUnescaping the wildcard is unescaped (encoded file names work) but
348+
// encoded separators and ".." segments are still rejected.
349+
func TestStatic_EnablePathUnescaping(t *testing.T) {
350+
fsys := fstest.MapFS{
351+
"hello world.txt": {Data: []byte("spaced")},
352+
"admin/secret.txt": {Data: []byte("TOP-SECRET")},
353+
}
354+
e := echo.New()
355+
g := e.Group("/files", StaticWithConfig(StaticConfig{Filesystem: fsys, EnablePathUnescaping: true}))
356+
g.GET("/*", func(c *echo.Context) error { return echo.ErrNotFound })
357+
358+
cases := []struct {
359+
target string
360+
wantCode int
361+
wantBody string
362+
}{
363+
{"/files/hello%20world.txt", http.StatusOK, "spaced"},
364+
{"/files/admin%2Fsecret.txt", http.StatusNotFound, ""},
365+
{"/files/public/%2E%2E/admin/secret.txt", http.StatusNotFound, ""},
366+
}
367+
for _, tc := range cases {
368+
req := httptest.NewRequest(http.MethodGet, tc.target, nil)
369+
rec := httptest.NewRecorder()
370+
e.ServeHTTP(rec, req)
371+
assert.Equal(t, tc.wantCode, rec.Code, "GET %s", tc.target)
372+
if tc.wantBody != "" {
373+
assert.Equal(t, tc.wantBody, rec.Body.String(), "GET %s", tc.target)
374+
}
375+
assert.NotContains(t, rec.Body.String(), "TOP-SECRET", "GET %s leaked protected file", tc.target)
376+
}
377+
}
378+
312379
func TestMustStaticWithConfig_panicsInvalidDirListTemplate(t *testing.T) {
313380
assert.Panics(t, func() {
314381
StaticWithConfig(StaticConfig{DirectoryListTemplate: `{{}`})

static_encoded_dotdot_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package echo
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
"testing/fstest"
8+
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func adminSecretFS() fstest.MapFS {
13+
return fstest.MapFS{
14+
"admin/secret.txt": {Data: []byte("TOP-SECRET")},
15+
"public/ok.txt": {Data: []byte("public")},
16+
"index.html": {Data: []byte("index")},
17+
}
18+
}
19+
20+
// Regression for GHSA-3pmx-cf9f-34xr: encoded/decoded ".." in a static wildcard must
21+
// not resolve a file across a route-level middleware guard. With unescaping disabled
22+
// by default the encoded form never decodes, and the dot-dot guard rejects any ".."
23+
// that the router (e.g. UseEscapedPathForMatching) decoded itself.
24+
func TestStaticFS_EncodedDotDotDoesNotBypassRoute(t *testing.T) {
25+
run := func(t *testing.T, e *Echo) {
26+
g := e.Group("/admin", func(next HandlerFunc) HandlerFunc {
27+
return func(c *Context) error { return c.String(http.StatusForbidden, "denied") }
28+
})
29+
g.GET("/*", func(c *Context) error { return c.String(http.StatusOK, "reached-protected-handler") })
30+
e.StaticFS("/", adminSecretFS())
31+
32+
cases := []struct {
33+
target string
34+
wantCode int
35+
}{
36+
{"/admin/secret.txt", http.StatusForbidden}, // protected route fires
37+
{"/public/%2E%2E/admin/secret.txt", http.StatusNotFound}, // high-sev: encoded dot-dot
38+
{"/public/%2e%2e/admin/secret.txt", http.StatusNotFound}, // lower-case hex
39+
{"/public%2F..%2Fadmin%2Fsecret.txt", http.StatusNotFound}, // encoded-slash variant
40+
{"/public/../admin/secret.txt", http.StatusNotFound}, // literal dot-dot
41+
{"/public/ok.txt", http.StatusOK}, // legitimate static file
42+
{"/index.html", http.StatusOK}, // legitimate static file
43+
}
44+
for _, tc := range cases {
45+
req := httptest.NewRequest(http.MethodGet, tc.target, nil)
46+
rec := httptest.NewRecorder()
47+
e.ServeHTTP(rec, req)
48+
assert.Equal(t, tc.wantCode, rec.Code, "GET %s", tc.target)
49+
assert.NotContains(t, rec.Body.String(), "TOP-SECRET", "GET %s leaked protected file", tc.target)
50+
}
51+
}
52+
53+
t.Run("default router", func(t *testing.T) { run(t, New()) })
54+
t.Run("UseEscapedPathForMatching", func(t *testing.T) {
55+
run(t, NewWithConfig(Config{Router: NewRouter(RouterConfig{UseEscapedPathForMatching: true})}))
56+
})
57+
}
58+
59+
// With EnablePathUnescapingStaticFiles the wildcard is unescaped (so encoded file
60+
// names work), but encoded separators and ".." segments are still rejected.
61+
func TestStaticFS_EnablePathUnescapingStaticFiles(t *testing.T) {
62+
fsys := fstest.MapFS{
63+
"hello world.txt": {Data: []byte("spaced")},
64+
"admin/secret.txt": {Data: []byte("TOP-SECRET")},
65+
}
66+
e := NewWithConfig(Config{EnablePathUnescapingStaticFiles: true})
67+
g := e.Group("/admin", func(next HandlerFunc) HandlerFunc {
68+
return func(c *Context) error { return c.String(http.StatusForbidden, "denied") }
69+
})
70+
g.GET("/*", func(c *Context) error { return c.String(http.StatusOK, "reached") })
71+
e.StaticFS("/", fsys)
72+
73+
cases := []struct {
74+
target string
75+
wantCode int
76+
wantBody string
77+
}{
78+
{"/hello%20world.txt", http.StatusOK, "spaced"}, // encoded space now decoded and served
79+
{"/admin%2Fsecret.txt", http.StatusNotFound, ""}, // encoded slash still rejected
80+
{"/public/%2E%2E/admin/secret.txt", http.StatusNotFound, ""}, // encoded dot-dot still rejected
81+
}
82+
for _, tc := range cases {
83+
req := httptest.NewRequest(http.MethodGet, tc.target, nil)
84+
rec := httptest.NewRecorder()
85+
e.ServeHTTP(rec, req)
86+
assert.Equal(t, tc.wantCode, rec.Code, "GET %s", tc.target)
87+
if tc.wantBody != "" {
88+
assert.Equal(t, tc.wantBody, rec.Body.String(), "GET %s", tc.target)
89+
}
90+
assert.NotContains(t, rec.Body.String(), "TOP-SECRET", "GET %s leaked protected file", tc.target)
91+
}
92+
}

0 commit comments

Comments
 (0)