Skip to content

Commit c58cc50

Browse files
committed
security (static middleware): fix bowser=true listing all file names from given filesystem root
1 parent ba10490 commit c58cc50

File tree

6 files changed

+95
-23
lines changed

6 files changed

+95
-23
lines changed

_fixture/dist/private.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
private file
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
readme in assets
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
file inside subfolder

_fixture/dist/public/index.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<h1>Hello from index</h1>

middleware/static.go

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,13 @@ const directoryListHTMLTemplate = `
118118
</header>
119119
<ul>
120120
{{ range .Files }}
121+
{{ $href := .Name }}{{ if ne $.Name "/" }}{{ $href = print $.Name "/" .Name }}{{ end }}
121122
<li>
122123
{{ if .Dir }}
123124
{{ $name := print .Name "/" }}
124-
<a class="dir" href="{{ $name }}">{{ $name }}</a>
125+
<a class="dir" href="{{ $href }}">{{ $name }}</a>
125126
{{ else }}
126-
<a class="file" href="{{ .Name }}">{{ .Name }}</a>
127+
<a class="file" href="{{ $href }}">{{ .Name }}</a>
127128
<span>{{ .Size }}</span>
128129
{{ end }}
129130
</li>
@@ -196,14 +197,15 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
196197
// 3. The "/" prefix forces absolute path interpretation, removing ".." components
197198
// 4. Backslashes are treated as literal characters (not path separators), preventing traversal
198199
// See static_windows.go for Go 1.20+ filepath.Clean compatibility notes
199-
name := path.Join(config.Root, path.Clean("/"+p)) // "/"+ for security
200+
requestedPath := path.Clean("/" + p) // "/"+ for security
201+
filePath := path.Join(config.Root, requestedPath)
200202

201203
if config.IgnoreBase {
202204
routePath := path.Base(strings.TrimRight(c.Path(), "/*"))
203205
baseURLPath := path.Base(p)
204206
if baseURLPath == routePath {
205-
i := strings.LastIndex(name, routePath)
206-
name = name[:i] + strings.Replace(name[i:], routePath, "", 1)
207+
i := strings.LastIndex(filePath, routePath)
208+
filePath = filePath[:i] + strings.Replace(filePath[i:], routePath, "", 1)
207209
}
208210
}
209211

@@ -212,7 +214,7 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
212214
currentFS = c.Echo().Filesystem
213215
}
214216

215-
file, err := currentFS.Open(name)
217+
file, err := currentFS.Open(filePath)
216218
if err != nil {
217219
if !isIgnorableOpenFileError(err) {
218220
return err
@@ -243,10 +245,10 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
243245
}
244246

245247
if info.IsDir() {
246-
index, err := currentFS.Open(path.Join(name, config.Index))
248+
index, err := currentFS.Open(path.Join(filePath, config.Index))
247249
if err != nil {
248250
if config.Browse {
249-
return listDir(dirListTemplate, name, currentFS, c.Response())
251+
return listDir(dirListTemplate, requestedPath, filePath, currentFS, c.Response())
250252
}
251253

252254
return next(c)
@@ -276,34 +278,36 @@ func serveFile(c *echo.Context, file fs.File, info os.FileInfo) error {
276278
return nil
277279
}
278280

279-
func listDir(t *template.Template, name string, filesystem fs.FS, res http.ResponseWriter) error {
281+
func listDir(t *template.Template, requestedPath string, pathInFs string, filesystem fs.FS, res http.ResponseWriter) error {
282+
files, err := fs.ReadDir(filesystem, pathInFs)
283+
if err != nil {
284+
return fmt.Errorf("static middleware failed to read directory for listing: %w", err)
285+
}
286+
280287
// Create directory index
281288
res.Header().Set(echo.HeaderContentType, echo.MIMETextHTMLCharsetUTF8)
282289
data := struct {
283290
Name string
284291
Files []any
285292
}{
286-
Name: name,
293+
Name: requestedPath,
287294
}
288-
err := fs.WalkDir(filesystem, ".", func(path string, d fs.DirEntry, err error) error {
289-
if err != nil {
290-
return err
291-
}
292295

293-
info, infoErr := d.Info()
294-
if infoErr != nil {
295-
return fmt.Errorf("static middleware list dir error when getting file info: %w", infoErr)
296+
for _, f := range files {
297+
var size int64
298+
if !f.IsDir() {
299+
info, err := f.Info()
300+
if err != nil {
301+
return fmt.Errorf("static middleware failed to get file info for listing: %w", err)
302+
}
303+
size = info.Size()
296304
}
305+
297306
data.Files = append(data.Files, struct {
298307
Name string
299308
Dir bool
300309
Size string
301-
}{d.Name(), d.IsDir(), format(info.Size())})
302-
303-
return nil
304-
})
305-
if err != nil {
306-
return err
310+
}{f.Name(), f.IsDir(), format(size)})
307311
}
308312

309313
return t.Execute(res, data)

middleware/static_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,3 +577,67 @@ func TestStatic_CustomFS(t *testing.T) {
577577
})
578578
}
579579
}
580+
581+
func TestStatic_DirectoryBrowsing(t *testing.T) {
582+
var testCases = []struct {
583+
name string
584+
givenConfig StaticConfig
585+
whenURL string
586+
expectContains string
587+
expectNotContains []string
588+
expectCode int
589+
}{
590+
{
591+
name: "ok, should return index.html contents from Root=public folder",
592+
givenConfig: StaticConfig{
593+
Root: "public",
594+
Filesystem: os.DirFS("../_fixture/dist"),
595+
Browse: true,
596+
},
597+
whenURL: "/",
598+
expectCode: http.StatusOK,
599+
expectContains: `<h1>Hello from index</h1>`,
600+
},
601+
{
602+
name: "ok, should return only subfolder folder listing from Root=public/assets",
603+
givenConfig: StaticConfig{
604+
Root: "public",
605+
Filesystem: os.DirFS("../_fixture/dist"),
606+
Browse: true,
607+
},
608+
whenURL: "/assets",
609+
expectCode: http.StatusOK,
610+
expectContains: `<a class="file" href="/assets/readme.md">readme.md</a>`,
611+
expectNotContains: []string{
612+
`<h1>Hello from index</h1>`, // should see the listing, not index.html contents
613+
`private.txt`, // file from the parent folder
614+
`subfolder.md`, // file from subfolder
615+
},
616+
},
617+
}
618+
for _, tc := range testCases {
619+
t.Run(tc.name, func(t *testing.T) {
620+
e := echo.New()
621+
622+
middlewareFunc, err := tc.givenConfig.ToMiddleware()
623+
assert.NoError(t, err)
624+
625+
e.Use(middlewareFunc)
626+
627+
req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil)
628+
rec := httptest.NewRecorder()
629+
630+
e.ServeHTTP(rec, req)
631+
632+
assert.Equal(t, tc.expectCode, rec.Code)
633+
634+
responseBody := rec.Body.String()
635+
if tc.expectContains != "" {
636+
assert.Contains(t, responseBody, tc.expectContains, "body should contain: "+tc.expectContains)
637+
}
638+
for _, notContains := range tc.expectNotContains {
639+
assert.NotContains(t, responseBody, notContains, "body should NOT contain: "+notContains)
640+
}
641+
})
642+
}
643+
}

0 commit comments

Comments
 (0)