Skip to content

Commit 6c16259

Browse files
committed
Fix directory traversal vulnerability under Windows in Static middleware when default Echo filesytem is used (effectively middleware.StaticConfig{Filesystem: nil})
1 parent 88d975a commit 6c16259

5 files changed

Lines changed: 62 additions & 51 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@
44

55
**Security**
66

7-
* Fix directory traversal vulnerability under Windows in Static middleware when default Echo filesytem is used (effectively `middleware.StaticConfig{Filesystem: nil}`).
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.
815

916

1017
## v5.0.2 - 2026-02-02

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
}

middleware/static_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ func TestStatic_DirectoryBrowsing(t *testing.T) {
473473
},
474474
whenURL: "/assets",
475475
expectCode: http.StatusOK,
476-
expectContains: `<a class="file" href="/assets/readme.md">readme.md</a>`,
476+
expectContains: `<a class="file" href="readme.md">readme.md</a>`,
477477
expectNotContains: []string{
478478
`<h1>Hello from index</h1>`, // should see the listing, not index.html contents
479479
`private.txt`, // file from the parent folder

middleware/static_windows.go

Lines changed: 0 additions & 34 deletions
This file was deleted.

0 commit comments

Comments
 (0)