From 1902fe55f4abea08142df6ce8b337a835b8b14fd Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 10:50:35 -0700 Subject: [PATCH 1/3] static: reject backslash dot segments, test non-validating custom fs.FS --- echo_fs.go | 9 +++++ echo_fs_test.go | 75 +++++++++++++++++++++++++++++++++++- middleware/static.go | 10 ++++- middleware/static_test.go | 81 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 2 deletions(-) diff --git a/echo_fs.go b/echo_fs.go index cd96a547d..904f31bbe 100644 --- a/echo_fs.go +++ b/echo_fs.go @@ -224,6 +224,15 @@ func hasDotOrEmptySegment(p string) bool { if segment == "" || segment == "." || segment == ".." { return true } + // A backslash is a literal character in fs.FS names, but a filesystem that wrongly treats it as a separator + // (for example one built on filepath.Join on Windows) would resolve `..\` outside its root. + if strings.Contains(segment, `\`) { + for part := range strings.SplitSeq(segment, `\`) { + if part == "." || part == ".." { + return true + } + } + } } return false } diff --git a/echo_fs_test.go b/echo_fs_test.go index b169e3ea4..920418224 100644 --- a/echo_fs_test.go +++ b/echo_fs_test.go @@ -4,14 +4,17 @@ package echo import ( - "github.com/stretchr/testify/assert" "io/fs" "net/http" "net/http/httptest" "os" + "path/filepath" "strings" "testing" "testing/fstest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestEcho_StaticFS(t *testing.T) { @@ -366,3 +369,73 @@ func TestStaticDirectoryHandler_encodedDotsWithPathUnescaping(t *testing.T) { }) } } + +// nonValidatingDirFS is a custom fs.FS that does not enforce fs.ValidPath, so a name with ".." escapes its root. Echo +// must never pass such a name to a user supplied filesystem. +type nonValidatingDirFS struct{ root string } + +func (f nonValidatingDirFS) Open(name string) (fs.File, error) { + return os.Open(filepath.Join(f.root, name)) +} + +func TestEcho_StaticFS_nonValidatingCustomFSCannotEscapeRoot(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, "public"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "public", "index.txt"), []byte("public"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "secret.txt"), []byte("secret"), 0o644)) + + for _, unescape := range []bool{false, true} { + e := New() + e.EnablePathUnescapingStaticFiles = unescape + e.StaticFS("/", nonValidatingDirFS{root: filepath.Join(dir, "public")}) + + for _, target := range []string{ + "/../secret.txt", + "/%2e%2e/secret.txt", + "/..%2fsecret.txt", + "/sub/../../secret.txt", + "/..%5csecret.txt", + `/..\secret.txt`, + } { + req := httptest.NewRequest(http.MethodGet, target, nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code, "%s unescape=%v", target, unescape) + assert.NotContains(t, rec.Body.String(), "secret", "%s unescape=%v", target, unescape) + } + + req := httptest.NewRequest(http.MethodGet, "/index.txt", nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "public", rec.Body.String()) + } +} + +func TestHasDotOrEmptySegment(t *testing.T) { + var testCases = []struct { + path string + expect bool + }{ + {path: "", expect: false}, + {path: "/", expect: false}, + {path: "/index.html", expect: false}, + {path: "/css/app.css", expect: false}, + {path: "/..", expect: true}, + {path: "/a/../b", expect: true}, + {path: "/a/./b", expect: true}, + {path: "/a//b", expect: true}, + {path: `/..\secret.txt`, expect: true}, + {path: `/a\..\b`, expect: true}, + {path: `/.\secret.txt`, expect: true}, + {path: `/dir\file.txt`, expect: false}, + {path: `/a\\b`, expect: false}, + {path: "/..%2fsecret.txt", expect: false}, // still encoded, only unsafe once unescaped + } + for _, tc := range testCases { + t.Run(tc.path, func(t *testing.T) { + assert.Equal(t, tc.expect, hasDotOrEmptySegment(tc.path)) + }) + } +} diff --git a/middleware/static.go b/middleware/static.go index ac36bee69..250eaf934 100644 --- a/middleware/static.go +++ b/middleware/static.go @@ -219,7 +219,6 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { // 2. path.Clean() provides platform-independent behavior for URL paths // 3. The "/" prefix forces absolute path interpretation, removing ".." components // 4. Backslashes are treated as literal characters (not path separators), preventing traversal - // See static_windows.go for Go 1.20+ filepath.Clean compatibility notes name := path.Join(config.Root, path.Clean("/"+p)) // "/"+ for security if config.IgnoreBase { @@ -333,6 +332,15 @@ func hasDotOrEmptySegment(p string) bool { if segment == "" || segment == "." || segment == ".." { return true } + // A backslash is a literal character in fs.FS names, but a filesystem that wrongly treats it as a separator + // (for example one built on filepath.Join on Windows) would resolve `..\` outside its root. + if strings.Contains(segment, `\`) { + for part := range strings.SplitSeq(segment, `\`) { + if part == "." || part == ".." { + return true + } + } + } } return false } diff --git a/middleware/static_test.go b/middleware/static_test.go index 63e408da0..b04391033 100644 --- a/middleware/static_test.go +++ b/middleware/static_test.go @@ -15,6 +15,7 @@ import ( "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestStatic(t *testing.T) { @@ -660,3 +661,83 @@ func TestStatic_HTML5WithUncleanPath(t *testing.T) { assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, "spa", rec.Body.String()) } + +// nonValidatingFS is a custom fs.FS that does not enforce fs.ValidPath, so a name with ".." escapes its root. Echo must +// never pass such a name to a user supplied filesystem. +type nonValidatingFS struct{ root string } + +func (f nonValidatingFS) Open(name string) (fs.File, error) { + return os.Open(filepath.Join(f.root, name)) +} + +func TestStatic_nonValidatingCustomFSCannotEscapeRoot(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, "public"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "public", "index.txt"), []byte("public"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "secret.txt"), []byte("secret"), 0o644)) + + targets := []string{ + "/../secret.txt", + "/%2e%2e/secret.txt", + "/..%2fsecret.txt", + "/sub/../../secret.txt", + "/..%5csecret.txt", + `/..\secret.txt`, + } + for _, group := range []string{"", "/static"} { + for _, unescape := range []bool{false, true} { + e := echo.New() + mw := StaticWithConfig(StaticConfig{ + Filesystem: http.FS(nonValidatingFS{root: filepath.Join(dir, "public")}), + EnablePathUnescaping: unescape, + }) + if group == "" { + e.Use(mw) + } else { + e.Group(group, mw) + } + + for _, target := range targets { + req := httptest.NewRequest(http.MethodGet, group+target, nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code, "%s unescape=%v", group+target, unescape) + assert.NotContains(t, rec.Body.String(), "secret", "%s unescape=%v", group+target, unescape) + } + + req := httptest.NewRequest(http.MethodGet, group+"/index.txt", nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, http.StatusOK, rec.Code, "group=%q unescape=%v", group, unescape) + assert.Equal(t, "public", rec.Body.String()) + } + } +} + +func TestHasDotOrEmptySegment(t *testing.T) { + var testCases = []struct { + path string + expect bool + }{ + {path: "", expect: false}, + {path: "/", expect: false}, + {path: "/index.html", expect: false}, + {path: "/css/app.css", expect: false}, + {path: "/..", expect: true}, + {path: "/a/../b", expect: true}, + {path: "/a/./b", expect: true}, + {path: "/a//b", expect: true}, + {path: `/..\secret.txt`, expect: true}, + {path: `/a\..\b`, expect: true}, + {path: `/.\secret.txt`, expect: true}, + {path: `/dir\file.txt`, expect: false}, + {path: `/a\\b`, expect: false}, + {path: "/..%2fsecret.txt", expect: false}, // still encoded, only unsafe once unescaped + } + for _, tc := range testCases { + t.Run(tc.path, func(t *testing.T) { + assert.Equal(t, tc.expect, hasDotOrEmptySegment(tc.path)) + }) + } +} From bcdb7f98cc0f785afd7f185077d552f7e764c1ac Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 10:55:36 -0700 Subject: [PATCH 2/3] static: test backslash hardening on every OS, add default-config vectors, fix comments --- echo_fs.go | 4 ++-- echo_fs_test.go | 9 ++++++++- middleware/static.go | 7 ++++--- middleware/static_test.go | 9 ++++++++- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/echo_fs.go b/echo_fs.go index 904f31bbe..e0905194f 100644 --- a/echo_fs.go +++ b/echo_fs.go @@ -211,8 +211,8 @@ func escapeControlChars(s string) string { return b.String() } -// hasDotOrEmptySegment reports whether URL path p has a ".", ".." or empty segment. A single leading and a single -// trailing slash are allowed. +// hasDotOrEmptySegment reports whether URL path p has a ".", ".." or empty segment, or a segment with a "." or ".." +// part between backslashes (e.g. `..\x`). A single leading and a single trailing slash are allowed. // Keep in sync with the copy in middleware/static.go. func hasDotOrEmptySegment(p string) bool { p = strings.TrimPrefix(p, "/") diff --git a/echo_fs_test.go b/echo_fs_test.go index 920418224..3b561eb40 100644 --- a/echo_fs_test.go +++ b/echo_fs_test.go @@ -375,7 +375,8 @@ func TestStaticDirectoryHandler_encodedDotsWithPathUnescaping(t *testing.T) { type nonValidatingDirFS struct{ root string } func (f nonValidatingDirFS) Open(name string) (fs.File, error) { - return os.Open(filepath.Join(f.root, name)) + // treat a backslash as a separator on every OS, like filepath.Join does on Windows + return os.Open(filepath.Join(f.root, filepath.FromSlash(strings.ReplaceAll(name, `\`, "/")))) } func TestEcho_StaticFS_nonValidatingCustomFSCannotEscapeRoot(t *testing.T) { @@ -395,6 +396,8 @@ func TestEcho_StaticFS_nonValidatingCustomFSCannotEscapeRoot(t *testing.T) { "/..%2fsecret.txt", "/sub/../../secret.txt", "/..%5csecret.txt", + "/..%5Csecret.txt", + "/a%5C..%5C..%5Csecret.txt", `/..\secret.txt`, } { req := httptest.NewRequest(http.MethodGet, target, nil) @@ -429,7 +432,11 @@ func TestHasDotOrEmptySegment(t *testing.T) { {path: `/..\secret.txt`, expect: true}, {path: `/a\..\b`, expect: true}, {path: `/.\secret.txt`, expect: true}, + {path: `/\..`, expect: true}, + {path: `/..\`, expect: true}, {path: `/dir\file.txt`, expect: false}, + {path: "/...", expect: false}, + {path: "/..foo", expect: false}, {path: `/a\\b`, expect: false}, {path: "/..%2fsecret.txt", expect: false}, // still encoded, only unsafe once unescaped } diff --git a/middleware/static.go b/middleware/static.go index 250eaf934..6d1000de0 100644 --- a/middleware/static.go +++ b/middleware/static.go @@ -218,7 +218,8 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { // 1. HTTP URLs always use forward slashes, regardless of server OS // 2. path.Clean() provides platform-independent behavior for URL paths // 3. The "/" prefix forces absolute path interpretation, removing ".." components - // 4. Backslashes are treated as literal characters (not path separators), preventing traversal + // 4. path.Clean() treats backslashes as literal characters; "."/".." parts between backslashes are rejected above + // for filesystems that wrongly treat a backslash as a separator name := path.Join(config.Root, path.Clean("/"+p)) // "/"+ for security if config.IgnoreBase { @@ -319,8 +320,8 @@ func listDir(t *template.Template, name string, dir http.File, res *echo.Respons return t.Execute(res, data) } -// hasDotOrEmptySegment reports whether URL path p has a ".", ".." or empty segment. A single leading and a single -// trailing slash are allowed. +// hasDotOrEmptySegment reports whether URL path p has a ".", ".." or empty segment, or a segment with a "." or ".." +// part between backslashes (e.g. `..\x`). A single leading and a single trailing slash are allowed. // Keep in sync with the copy in echo_fs.go. func hasDotOrEmptySegment(p string) bool { p = strings.TrimPrefix(p, "/") diff --git a/middleware/static_test.go b/middleware/static_test.go index b04391033..8f45647ba 100644 --- a/middleware/static_test.go +++ b/middleware/static_test.go @@ -667,7 +667,8 @@ func TestStatic_HTML5WithUncleanPath(t *testing.T) { type nonValidatingFS struct{ root string } func (f nonValidatingFS) Open(name string) (fs.File, error) { - return os.Open(filepath.Join(f.root, name)) + // treat a backslash as a separator on every OS, like filepath.Join does on Windows + return os.Open(filepath.Join(f.root, filepath.FromSlash(strings.ReplaceAll(name, `\`, "/")))) } func TestStatic_nonValidatingCustomFSCannotEscapeRoot(t *testing.T) { @@ -682,6 +683,8 @@ func TestStatic_nonValidatingCustomFSCannotEscapeRoot(t *testing.T) { "/..%2fsecret.txt", "/sub/../../secret.txt", "/..%5csecret.txt", + "/..%5Csecret.txt", + "/a%5C..%5C..%5Csecret.txt", `/..\secret.txt`, } for _, group := range []string{"", "/static"} { @@ -731,7 +734,11 @@ func TestHasDotOrEmptySegment(t *testing.T) { {path: `/..\secret.txt`, expect: true}, {path: `/a\..\b`, expect: true}, {path: `/.\secret.txt`, expect: true}, + {path: `/\..`, expect: true}, + {path: `/..\`, expect: true}, {path: `/dir\file.txt`, expect: false}, + {path: "/...", expect: false}, + {path: "/..foo", expect: false}, {path: `/a\\b`, expect: false}, {path: "/..%2fsecret.txt", expect: false}, // still encoded, only unsafe once unescaped } From 94dfbae9428b086cea4577dec7a3513479d7b911 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 10:56:32 -0700 Subject: [PATCH 3/3] static: keep static_windows.go reference (exists on v4) --- middleware/static.go | 1 + 1 file changed, 1 insertion(+) diff --git a/middleware/static.go b/middleware/static.go index 6d1000de0..834067ac5 100644 --- a/middleware/static.go +++ b/middleware/static.go @@ -220,6 +220,7 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { // 3. The "/" prefix forces absolute path interpretation, removing ".." components // 4. path.Clean() treats backslashes as literal characters; "."/".." parts between backslashes are rejected above // for filesystems that wrongly treat a backslash as a separator + // See static_windows.go for Go 1.20+ filepath.Clean compatibility notes name := path.Join(config.Root, path.Clean("/"+p)) // "/"+ for security if config.IgnoreBase {