Skip to content

Commit 4422e3b

Browse files
lammellammel
authored
Merge pull request #1718 from little-cui/master
Fix static directory traversal security vulnerability for Windows
2 parents e7741d4 + 1beaf09 commit 4422e3b

File tree

3 files changed

+99
-40
lines changed

3 files changed

+99
-40
lines changed

‎echo.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ import (
4949
"net/http"
5050
"net/url"
5151
"os"
52-
"path"
5352
"path/filepath"
5453
"reflect"
5554
"runtime"
@@ -486,7 +485,7 @@ func (common) static(prefix, root string, get func(string, HandlerFunc, ...Middl
486485
return err
487486
}
488487

489-
name := filepath.Join(root, path.Clean("/"+p)) // "/"+ for security
488+
name := filepath.Join(root, filepath.Clean("/"+p)) // "/"+ for security
490489
fi, err := os.Stat(name)
491490
if err != nil {
492491
// The access path does not exist

‎echo_test.go

Lines changed: 97 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -60,45 +60,105 @@ func TestEcho(t *testing.T) {
6060
}
6161

6262
func TestEchoStatic(t *testing.T) {
63-
e := New()
64-
65-
assert := assert.New(t)
66-
67-
// OK
68-
e.Static("/images", "_fixture/images")
69-
c, b := request(http.MethodGet, "/images/walle.png", e)
70-
assert.Equal(http.StatusOK, c)
71-
assert.NotEmpty(b)
72-
73-
// No file
74-
e.Static("/images", "_fixture/scripts")
75-
c, _ = request(http.MethodGet, "/images/bolt.png", e)
76-
assert.Equal(http.StatusNotFound, c)
77-
78-
// Directory
79-
e.Static("/images", "_fixture/images")
80-
c, _ = request(http.MethodGet, "/images/", e)
81-
assert.Equal(http.StatusNotFound, c)
82-
83-
// Directory Redirect
84-
e.Static("/", "_fixture")
85-
req := httptest.NewRequest(http.MethodGet, "/folder", nil)
86-
rec := httptest.NewRecorder()
87-
e.ServeHTTP(rec, req)
88-
assert.Equal(http.StatusMovedPermanently, rec.Code)
89-
assert.Equal("/folder/", rec.HeaderMap["Location"][0])
90-
91-
// Directory with index.html
92-
e.Static("/", "_fixture")
93-
c, r := request(http.MethodGet, "/", e)
94-
assert.Equal(http.StatusOK, c)
95-
assert.Equal(true, strings.HasPrefix(r, "<!doctype html>"))
63+
var testCases = []struct {
64+
name string
65+
givenPrefix string
66+
givenRoot string
67+
whenURL string
68+
expectStatus int
69+
expectHeaderLocation string
70+
expectBodyStartsWith string
71+
}{
72+
{
73+
name: "ok",
74+
givenPrefix: "/images",
75+
givenRoot: "_fixture/images",
76+
whenURL: "/images/walle.png",
77+
expectStatus: http.StatusOK,
78+
expectBodyStartsWith: string([]byte{0x89, 0x50, 0x4e, 0x47}),
79+
},
80+
{
81+
name: "No file",
82+
givenPrefix: "/images",
83+
givenRoot: "_fixture/scripts",
84+
whenURL: "/images/bolt.png",
85+
expectStatus: http.StatusNotFound,
86+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
87+
},
88+
{
89+
name: "Directory",
90+
givenPrefix: "/images",
91+
givenRoot: "_fixture/images",
92+
whenURL: "/images/",
93+
expectStatus: http.StatusNotFound,
94+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
95+
},
96+
{
97+
name: "Directory Redirect",
98+
givenPrefix: "/",
99+
givenRoot: "_fixture",
100+
whenURL: "/folder",
101+
expectStatus: http.StatusMovedPermanently,
102+
expectHeaderLocation: "/folder/",
103+
expectBodyStartsWith: "",
104+
},
105+
{
106+
name: "Directory with index.html",
107+
givenPrefix: "/",
108+
givenRoot: "_fixture",
109+
whenURL: "/",
110+
expectStatus: http.StatusOK,
111+
expectBodyStartsWith: "<!doctype html>",
112+
},
113+
{
114+
name: "Sub-directory with index.html",
115+
givenPrefix: "/",
116+
givenRoot: "_fixture",
117+
whenURL: "/folder/",
118+
expectStatus: http.StatusOK,
119+
expectBodyStartsWith: "<!doctype html>",
120+
},
121+
{
122+
name: "do not allow directory traversal (backslash - windows separator)",
123+
givenPrefix: "/",
124+
givenRoot: "_fixture/",
125+
whenURL: `/..\\middleware/basic_auth.go`,
126+
expectStatus: http.StatusNotFound,
127+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
128+
},
129+
{
130+
name: "do not allow directory traversal (slash - unix separator)",
131+
givenPrefix: "/",
132+
givenRoot: "_fixture/",
133+
whenURL: `/../middleware/basic_auth.go`,
134+
expectStatus: http.StatusNotFound,
135+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
136+
},
137+
}
96138

97-
// Sub-directory with index.html
98-
c, r = request(http.MethodGet, "/folder/", e)
99-
assert.Equal(http.StatusOK, c)
100-
assert.Equal(true, strings.HasPrefix(r, "<!doctype html>"))
139+
for _, tc := range testCases {
140+
t.Run(tc.name, func(t *testing.T) {
141+
e := New()
142+
e.Static(tc.givenPrefix, tc.givenRoot)
143+
req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil)
144+
rec := httptest.NewRecorder()
145+
e.ServeHTTP(rec, req)
146+
assert.Equal(t, tc.expectStatus, rec.Code)
147+
body := rec.Body.String()
148+
if tc.expectBodyStartsWith != "" {
149+
assert.True(t, strings.HasPrefix(body, tc.expectBodyStartsWith))
150+
} else {
151+
assert.Equal(t, "", body)
152+
}
101153

154+
if tc.expectHeaderLocation != "" {
155+
assert.Equal(t, tc.expectHeaderLocation, rec.Result().Header["Location"][0])
156+
} else {
157+
_, ok := rec.Result().Header["Location"]
158+
assert.False(t, ok)
159+
}
160+
})
161+
}
102162
}
103163

104164
func TestEchoFile(t *testing.T) {

‎middleware/static.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc {
167167
if err != nil {
168168
return
169169
}
170-
name := filepath.Join(config.Root, path.Clean("/"+p)) // "/"+ for security
170+
name := filepath.Join(config.Root, filepath.Clean("/"+p)) // "/"+ for security
171171

172172
if config.IgnoreBase {
173173
routePath := path.Base(strings.TrimRight(c.Path(), "/*"))

0 commit comments

Comments
 (0)