From 1beaf09740e7e3ed4f6bfdd753121c03946a1142 Mon Sep 17 00:00:00 2001 From: little-cui Date: Sun, 13 Dec 2020 21:49:11 +0800 Subject: [PATCH] Bug Fix: Directory Traversal --- echo.go | 3 +- echo_test.go | 134 +++++++++++++++++++++++++++++++------------ middleware/static.go | 2 +- 3 files changed, 99 insertions(+), 40 deletions(-) diff --git a/echo.go b/echo.go index 381604180..d284ff396 100644 --- a/echo.go +++ b/echo.go @@ -49,7 +49,6 @@ import ( "net/http" "net/url" "os" - "path" "path/filepath" "reflect" "runtime" @@ -486,7 +485,7 @@ func (common) static(prefix, root string, get func(string, HandlerFunc, ...Middl return err } - name := filepath.Join(root, path.Clean("/"+p)) // "/"+ for security + name := filepath.Join(root, filepath.Clean("/"+p)) // "/"+ for security fi, err := os.Stat(name) if err != nil { // The access path does not exist diff --git a/echo_test.go b/echo_test.go index bf3ecfe91..a6071e12a 100644 --- a/echo_test.go +++ b/echo_test.go @@ -60,45 +60,105 @@ func TestEcho(t *testing.T) { } func TestEchoStatic(t *testing.T) { - e := New() - - assert := assert.New(t) - - // OK - e.Static("/images", "_fixture/images") - c, b := request(http.MethodGet, "/images/walle.png", e) - assert.Equal(http.StatusOK, c) - assert.NotEmpty(b) - - // No file - e.Static("/images", "_fixture/scripts") - c, _ = request(http.MethodGet, "/images/bolt.png", e) - assert.Equal(http.StatusNotFound, c) - - // Directory - e.Static("/images", "_fixture/images") - c, _ = request(http.MethodGet, "/images/", e) - assert.Equal(http.StatusNotFound, c) - - // Directory Redirect - e.Static("/", "_fixture") - req := httptest.NewRequest(http.MethodGet, "/folder", nil) - rec := httptest.NewRecorder() - e.ServeHTTP(rec, req) - assert.Equal(http.StatusMovedPermanently, rec.Code) - assert.Equal("/folder/", rec.HeaderMap["Location"][0]) - - // Directory with index.html - e.Static("/", "_fixture") - c, r := request(http.MethodGet, "/", e) - assert.Equal(http.StatusOK, c) - assert.Equal(true, strings.HasPrefix(r, "")) + var testCases = []struct { + name string + givenPrefix string + givenRoot string + whenURL string + expectStatus int + expectHeaderLocation string + expectBodyStartsWith string + }{ + { + name: "ok", + givenPrefix: "/images", + givenRoot: "_fixture/images", + whenURL: "/images/walle.png", + expectStatus: http.StatusOK, + expectBodyStartsWith: string([]byte{0x89, 0x50, 0x4e, 0x47}), + }, + { + name: "No file", + givenPrefix: "/images", + givenRoot: "_fixture/scripts", + whenURL: "/images/bolt.png", + expectStatus: http.StatusNotFound, + expectBodyStartsWith: "{\"message\":\"Not Found\"}\n", + }, + { + name: "Directory", + givenPrefix: "/images", + givenRoot: "_fixture/images", + whenURL: "/images/", + expectStatus: http.StatusNotFound, + expectBodyStartsWith: "{\"message\":\"Not Found\"}\n", + }, + { + name: "Directory Redirect", + givenPrefix: "/", + givenRoot: "_fixture", + whenURL: "/folder", + expectStatus: http.StatusMovedPermanently, + expectHeaderLocation: "/folder/", + expectBodyStartsWith: "", + }, + { + name: "Directory with index.html", + givenPrefix: "/", + givenRoot: "_fixture", + whenURL: "/", + expectStatus: http.StatusOK, + expectBodyStartsWith: "", + }, + { + name: "Sub-directory with index.html", + givenPrefix: "/", + givenRoot: "_fixture", + whenURL: "/folder/", + expectStatus: http.StatusOK, + expectBodyStartsWith: "", + }, + { + name: "do not allow directory traversal (backslash - windows separator)", + givenPrefix: "/", + givenRoot: "_fixture/", + whenURL: `/..\\middleware/basic_auth.go`, + expectStatus: http.StatusNotFound, + expectBodyStartsWith: "{\"message\":\"Not Found\"}\n", + }, + { + name: "do not allow directory traversal (slash - unix separator)", + givenPrefix: "/", + givenRoot: "_fixture/", + whenURL: `/../middleware/basic_auth.go`, + expectStatus: http.StatusNotFound, + expectBodyStartsWith: "{\"message\":\"Not Found\"}\n", + }, + } - // Sub-directory with index.html - c, r = request(http.MethodGet, "/folder/", e) - assert.Equal(http.StatusOK, c) - assert.Equal(true, strings.HasPrefix(r, "")) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + e := New() + e.Static(tc.givenPrefix, tc.givenRoot) + req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, tc.expectStatus, rec.Code) + body := rec.Body.String() + if tc.expectBodyStartsWith != "" { + assert.True(t, strings.HasPrefix(body, tc.expectBodyStartsWith)) + } else { + assert.Equal(t, "", body) + } + if tc.expectHeaderLocation != "" { + assert.Equal(t, tc.expectHeaderLocation, rec.Result().Header["Location"][0]) + } else { + _, ok := rec.Result().Header["Location"] + assert.False(t, ok) + } + }) + } } func TestEchoFile(t *testing.T) { diff --git a/middleware/static.go b/middleware/static.go index 58b7890a4..ae79cb5fa 100644 --- a/middleware/static.go +++ b/middleware/static.go @@ -167,7 +167,7 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc { if err != nil { return } - name := filepath.Join(config.Root, path.Clean("/"+p)) // "/"+ for security + name := filepath.Join(config.Root, filepath.Clean("/"+p)) // "/"+ for security if config.IgnoreBase { routePath := path.Base(strings.TrimRight(c.Path(), "/*"))