Skip to content

Commit b1d4430

Browse files
aldasaldas
authored
Merge pull request #2891 from aldas/fix_staticmw
Fix directory traversal vulnerability under Windows in Static middleware when default Echo filesystem is used. Reported by @shblue21. This applies to cases when: - Windows is used as OS - `middleware.StaticConfig.Filesystem` is `nil` (default) - `echo.Filesystem` is has not been set explicitly (default) Exposure is restricted to the active process working directory and its subfolders.
2 parents 09ccfba + 48f25a6 commit b1d4430

File tree

16 files changed

+195
-286
lines changed

16 files changed

+195
-286
lines changed

‎CHANGELOG.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
11
# Changelog
22

3+
## v5.0.3 - 2026-02-06
4+
5+
**Security**
6+
7+
* Fix directory traversal vulnerability under Windows in Static middleware when default Echo filesystem is used. Reported by @shblue21.
8+
9+
This applies to cases when:
10+
- Windows is used as OS
11+
- `middleware.StaticConfig.Filesystem` is `nil` (default)
12+
- `echo.Filesystem` is has not been set explicitly (default)
13+
14+
Exposure is restricted to the active process working directory and its subfolders.
15+
16+
317
## v5.0.2 - 2026-02-02
418

519
**Security**

‎_fixture/dist/public/test.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test.txt contents

‎context.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"net"
1616
"net/http"
1717
"net/url"
18+
"path"
1819
"path/filepath"
1920
"strings"
2021
"sync"
@@ -579,6 +580,7 @@ func (c *Context) FileFS(file string, filesystem fs.FS) error {
579580
}
580581

581582
func fsFile(c *Context, file string, filesystem fs.FS) error {
583+
file = path.Clean(file) // `os.Open` and `os.DirFs.Open()` behave differently, later does not like ``, `.`, `..` at all, but we allowed those now need to clean
582584
f, err := filesystem.Open(file)
583585
if err != nil {
584586
return ErrNotFound

‎echo.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -785,14 +785,11 @@ func newDefaultFS() *defaultFS {
785785
dir, _ := os.Getwd()
786786
return &defaultFS{
787787
prefix: dir,
788-
fs: nil,
788+
fs: os.DirFS(dir),
789789
}
790790
}
791791

792792
func (fs defaultFS) Open(name string) (fs.File, error) {
793-
if fs.fs == nil {
794-
return os.Open(name) // #nosec G304
795-
}
796793
return fs.fs.Open(name)
797794
}
798795

‎group_test.go‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -467,13 +467,14 @@ func TestGroup_Static(t *testing.T) {
467467

468468
func TestGroup_StaticMultiTest(t *testing.T) {
469469
var testCases = []struct {
470-
name string
471-
givenPrefix string
472-
givenRoot string
473-
whenURL string
474-
expectHeaderLocation string
475-
expectBodyStartsWith string
476-
expectStatus int
470+
name string
471+
givenPrefix string
472+
givenRoot string
473+
whenURL string
474+
expectHeaderLocation string
475+
expectBodyStartsWith string
476+
expectBodyNotContains string
477+
expectStatus int
477478
}{
478479
{
479480
name: "ok",
@@ -582,6 +583,22 @@ func TestGroup_StaticMultiTest(t *testing.T) {
582583
expectStatus: http.StatusOK,
583584
expectBodyStartsWith: "<!doctype html>",
584585
},
586+
{
587+
name: "nok, URL encoded path traversal (single encoding, slash - unix separator)",
588+
givenRoot: "_fixture/dist/public",
589+
whenURL: "/%2e%2e%2fprivate.txt",
590+
expectStatus: http.StatusNotFound,
591+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
592+
expectBodyNotContains: `private file`,
593+
},
594+
{
595+
name: "nok, URL encoded path traversal (single encoding, backslash - windows separator)",
596+
givenRoot: "_fixture/dist/public",
597+
whenURL: "/%2e%2e%5cprivate.txt",
598+
expectStatus: http.StatusNotFound,
599+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
600+
expectBodyNotContains: `private file`,
601+
},
585602
{
586603
name: "do not allow directory traversal (backslash - windows separator)",
587604
givenPrefix: "/",
@@ -618,6 +635,9 @@ func TestGroup_StaticMultiTest(t *testing.T) {
618635
} else {
619636
assert.Equal(t, "", body)
620637
}
638+
if tc.expectBodyNotContains != "" {
639+
assert.NotContains(t, body, tc.expectBodyNotContains)
640+
}
621641

622642
if tc.expectHeaderLocation != "" {
623643
assert.Equal(t, tc.expectHeaderLocation, rec.Result().Header["Location"][0])

‎middleware/static.go‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"path"
1616
"strconv"
1717
"strings"
18+
"sync"
1819

1920
"github.com/labstack/echo/v5"
2021
)
@@ -118,13 +119,12 @@ const directoryListHTMLTemplate = `
118119
</header>
119120
<ul>
120121
{{ range .Files }}
121-
{{ $href := .Name }}{{ if ne $.Name "/" }}{{ $href = print $.Name "/" .Name }}{{ end }}
122122
<li>
123123
{{ if .Dir }}
124124
{{ $name := print .Name "/" }}
125-
<a class="dir" href="{{ $href }}">{{ $name }}</a>
125+
<a class="dir" href="{{ $name }}">{{ $name }}</a>
126126
{{ else }}
127-
<a class="file" href="{{ $href }}">{{ .Name }}</a>
127+
<a class="file" href="{{ .Name }}">{{ .Name }}</a>
128128
<span>{{ .Size }}</span>
129129
{{ end }}
130130
</li>
@@ -157,7 +157,10 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
157157
// Defaults
158158
if config.Root == "" {
159159
config.Root = "." // For security we want to restrict to CWD.
160+
} else {
161+
config.Root = path.Clean(config.Root) // fs.Open is very picky about ``, `.`, `..` in paths, so remove some of them up.
160162
}
163+
161164
if config.Skipper == nil {
162165
config.Skipper = DefaultStaticConfig.Skipper
163166
}
@@ -173,6 +176,19 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
173176
return nil, fmt.Errorf("echo static middleware directory list template parsing error: %w", tErr)
174177
}
175178

179+
var once *sync.Once
180+
var fsErr error
181+
currentFS := config.Filesystem
182+
if config.Filesystem == nil {
183+
once = &sync.Once{}
184+
} else if config.Root != "." {
185+
tmpFs, fErr := fs.Sub(config.Filesystem, path.Join(".", config.Root))
186+
if fErr != nil {
187+
return nil, fmt.Errorf("static middleware failed to create sub-filesystem from config.Root, error: %w", fErr)
188+
}
189+
currentFS = tmpFs
190+
}
191+
176192
return func(next echo.HandlerFunc) echo.HandlerFunc {
177193
return func(c *echo.Context) (err error) {
178194
if config.Skipper(c) {
@@ -197,8 +213,7 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
197213
// 3. The "/" prefix forces absolute path interpretation, removing ".." components
198214
// 4. Backslashes are treated as literal characters (not path separators), preventing traversal
199215
// See static_windows.go for Go 1.20+ filepath.Clean compatibility notes
200-
requestedPath := path.Clean("/" + p) // "/"+ for security
201-
filePath := path.Join(config.Root, requestedPath)
216+
filePath := path.Clean("./" + p)
202217

203218
if config.IgnoreBase {
204219
routePath := path.Base(strings.TrimRight(c.Path(), "/*"))
@@ -209,9 +224,17 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
209224
}
210225
}
211226

212-
currentFS := config.Filesystem
213-
if currentFS == nil {
214-
currentFS = c.Echo().Filesystem
227+
if once != nil {
228+
once.Do(func() {
229+
if tmp, tmpErr := fs.Sub(c.Echo().Filesystem, config.Root); tmpErr != nil {
230+
fsErr = fmt.Errorf("static middleware failed to create sub-filesystem: %w", tmpErr)
231+
} else {
232+
currentFS = tmp
233+
}
234+
})
235+
if fsErr != nil {
236+
return fsErr
237+
}
215238
}
216239

217240
file, err := currentFS.Open(filePath)
@@ -231,7 +254,7 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
231254
return err
232255
}
233256
// is case HTML5 mode is enabled + echo 404 we serve index to the client
234-
file, err = currentFS.Open(path.Join(config.Root, config.Index))
257+
file, err = currentFS.Open(config.Index)
235258
if err != nil {
236259
return err
237260
}
@@ -248,7 +271,7 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
248271
index, err := currentFS.Open(path.Join(filePath, config.Index))
249272
if err != nil {
250273
if config.Browse {
251-
return listDir(dirListTemplate, requestedPath, filePath, currentFS, c.Response())
274+
return listDir(dirListTemplate, filePath, currentFS, c.Response())
252275
}
253276

254277
return next(c)
@@ -278,7 +301,7 @@ func serveFile(c *echo.Context, file fs.File, info os.FileInfo) error {
278301
return nil
279302
}
280303

281-
func listDir(t *template.Template, requestedPath string, pathInFs string, filesystem fs.FS, res http.ResponseWriter) error {
304+
func listDir(t *template.Template, pathInFs string, filesystem fs.FS, res http.ResponseWriter) error {
282305
files, err := fs.ReadDir(filesystem, pathInFs)
283306
if err != nil {
284307
return fmt.Errorf("static middleware failed to read directory for listing: %w", err)
@@ -290,7 +313,7 @@ func listDir(t *template.Template, requestedPath string, pathInFs string, filesy
290313
Name string
291314
Files []any
292315
}{
293-
Name: requestedPath,
316+
Name: pathInFs,
294317
}
295318

296319
for _, f := range files {

‎middleware/static_other.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,30 @@
11
// SPDX-License-Identifier: MIT
22
// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors
33

4-
//go:build !windows
5-
64
package middleware
75

86
import (
7+
"errors"
8+
"io/fs"
99
"os"
1010
)
1111

1212
// We ignore these errors as there could be handler that matches request path.
1313
func isIgnorableOpenFileError(err error) bool {
14-
return os.IsNotExist(err)
14+
if os.IsNotExist(err) {
15+
return true
16+
}
17+
// As of Go 1.20 Windows path checks are more strict on the provided path and considers [UNC](https://en.wikipedia.org/wiki/Path_(computing)#UNC)
18+
// paths with missing host etc parts as invalid. Previously it would result you `fs.ErrNotExist`.
19+
// Also `fs.Open` on all OSes does not accept ``, `.`, `..` at all.
20+
//
21+
// so we need to treat those errors the same as `fs.ErrNotExists` so we can continue handling
22+
// errors in the middleware/handler chain. Otherwise we might end up with status 500 instead of finding a route
23+
// or return 404 not found.
24+
var pErr *fs.PathError
25+
if errors.As(err, &pErr) {
26+
err = pErr.Err
27+
return err.Error() == "invalid argument"
28+
}
29+
return false
1530
}

0 commit comments

Comments
 (0)