From 35671a97d6e7ccb426be8a6204b3d439fe367c36 Mon Sep 17 00:00:00 2001 From: Asher Myers <43706372+ashermyers@users.noreply.github.com> Date: Tue, 20 Aug 2024 02:28:28 -0400 Subject: [PATCH 1/6] Update static.go --- middleware/static/static.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/middleware/static/static.go b/middleware/static/static.go index 6cbdbd3d22..22db90562d 100644 --- a/middleware/static/static.go +++ b/middleware/static/static.go @@ -65,7 +65,7 @@ func New(root string, cfg ...Config) fiber.Handler { GenerateIndexPages: config.Browse, AcceptByteRange: config.ByteRange, Compress: config.Compress, - CompressBrotli: config.Compress, // Brotli compression won't work without this + CompressBrotli: config.Compress, CompressedFileSuffixes: c.App().Config().CompressedFileSuffixes, CacheDuration: config.CacheDuration, SkipCache: config.CacheDuration < 0, @@ -102,6 +102,19 @@ func New(root string, cfg ...Config) fiber.Handler { path = append([]byte("/"), path...) } + // Perform explicit path validation + absRoot, err := filepath.Abs(root) + if err != nil { + fctx.Response.SetStatusCode(fiber.StatusInternalServerError) + return nil + } + + absPath, err := filepath.Abs(filepath.Join(absRoot, string(path))) + if err != nil || !strings.HasPrefix(absPath, absRoot) { + fctx.Response.SetStatusCode(fiber.StatusForbidden) + return nil + } + return path } From 63afcdfdc323c08b03fcb546bc8eba73194dce7f Mon Sep 17 00:00:00 2001 From: Asher Myers <43706372+ashermyers@users.noreply.github.com> Date: Tue, 20 Aug 2024 02:29:20 -0400 Subject: [PATCH 2/6] Update static.go --- middleware/static/static.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/middleware/static/static.go b/middleware/static/static.go index 22db90562d..d54ae1ee68 100644 --- a/middleware/static/static.go +++ b/middleware/static/static.go @@ -65,7 +65,7 @@ func New(root string, cfg ...Config) fiber.Handler { GenerateIndexPages: config.Browse, AcceptByteRange: config.ByteRange, Compress: config.Compress, - CompressBrotli: config.Compress, + CompressBrotli: config.Compress, // Brotli compression won't work without this CompressedFileSuffixes: c.App().Config().CompressedFileSuffixes, CacheDuration: config.CacheDuration, SkipCache: config.CacheDuration < 0, From cc6ed88cf46487b6472f8fd8f426b6d2f0fc6f92 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Sun, 8 Dec 2024 23:47:51 -0500 Subject: [PATCH 3/6] Add more checks. Add extensive testing of traversal paths --- middleware/static/static.go | 15 +++- middleware/static/static_test.go | 113 ++++++++++++++++++++++++++++++- 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/middleware/static/static.go b/middleware/static/static.go index 7aea51a827..96830e22cc 100644 --- a/middleware/static/static.go +++ b/middleware/static/static.go @@ -98,6 +98,7 @@ func New(root string, cfg ...Config) fiber.Handler { } } + // Add a leading slash if missing if len(path) > 0 && path[0] != '/' { path = append([]byte("/"), path...) } @@ -109,13 +110,21 @@ func New(root string, cfg ...Config) fiber.Handler { return nil } - absPath, err := filepath.Abs(filepath.Join(absRoot, string(path))) - if err != nil || !strings.HasPrefix(absPath, absRoot) { + // Replace backslashes with slashes + safePath := strings.ReplaceAll(string(path), "\\", "/") + + // Clean the path and resolve it against the root + cleanPath := filepath.Clean("/" + safePath) + absPath := filepath.Join(absRoot, cleanPath) + relPath, err := filepath.Rel(absRoot, absPath) + + // Check if the resolved path is within the root + if err != nil || strings.HasPrefix(relPath, "..") { fctx.Response.SetStatusCode(fiber.StatusForbidden) return nil } - return path + return []byte(cleanPath) } maxAge := config.MaxAge diff --git a/middleware/static/static_test.go b/middleware/static/static_test.go index 4e1d7a96d8..71caf1a857 100644 --- a/middleware/static/static_test.go +++ b/middleware/static/static_test.go @@ -412,7 +412,7 @@ func Test_Static_Next(t *testing.T) { func Test_Route_Static_Root(t *testing.T) { t.Parallel() - dir := "../../.github/testdata/fs/css" + dir := "../../.github/testdata/fs/css" //nolint:goconst // test app := fiber.New() app.Get("/*", New(dir, Config{ Browse: true, @@ -850,3 +850,114 @@ func Test_Static_Compress_WithFileSuffixes(t *testing.T) { require.NoError(t, err, "File should exist") } } + +func Test_Static_PathTraversal(t *testing.T) { + t.Parallel() + app := fiber.New() + + // Serve only from "../../.github/testdata/fs/css" + // This directory should contain `style.css` but not `index.html` or anything above it. + rootDir := "../../.github/testdata/fs/css" + app.Get("/*", New(rootDir)) + + // A valid request: should succeed + validReq := httptest.NewRequest(fiber.MethodGet, "/style.css", nil) + validResp, err := app.Test(validReq) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, 200, validResp.StatusCode, "Status code") + require.Equal(t, fiber.MIMETextCSSCharsetUTF8, validResp.Header.Get(fiber.HeaderContentType)) + validBody, err := io.ReadAll(validResp.Body) + require.NoError(t, err, "app.Test(req)") + require.Contains(t, string(validBody), "color") + + // Helper function to assert that a given path is blocked. + // Blocked can mean different status codes depending on what triggered the block. + // We'll accept 400 or 404 as "blocked" statuses: + // - 404 is the expected blocked response in most cases. + // - 400 might occur if fasthttp rejects the request before it's even processed (e.g., null bytes). + assertTraversalBlocked := func(path string) { + req := httptest.NewRequest(fiber.MethodGet, path, nil) + resp, err := app.Test(req) + require.NoError(t, err, "app.Test(req)") + + status := resp.StatusCode + require.Truef(t, status == 400 || status == 404, + "Status code for path traversal %s should be 400 or 404, got %d", path, status) + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + // If we got a 404, we expect the "Cannot GET" message because that's how fiber handles NotFound by default. + if status == 404 { + require.Contains(t, string(body), "Cannot GET", + "Blocked traversal should have a Cannot GET message for %s", path) + } else { + require.Contains(t, string(body), "Are you a hacker?", + "Blocked traversal should have a Cannot GET message for %s", path) + } + } + + // Basic attempts to escape the directory + assertTraversalBlocked("/index.html..") + assertTraversalBlocked("/style.css..") + assertTraversalBlocked("/../index.html") + assertTraversalBlocked("/../../index.html") + assertTraversalBlocked("/../../../index.html") + + // Attempts with double slashes + assertTraversalBlocked("//../index.html") + assertTraversalBlocked("/..//index.html") + + // Encoded attempts: `%2e` is '.' and `%2f` is '/' + assertTraversalBlocked("/..%2findex.html") // ../index.html + assertTraversalBlocked("/%2e%2e/index.html") // ../index.html + assertTraversalBlocked("/%2e%2e%2f%2e%2e/secret") // ../../../secret + + // Mixed encoded and normal attempts + assertTraversalBlocked("/%2e%2e/../index.html") // ../../index.html + assertTraversalBlocked("/..%2f..%2fsecret.json") // ../../../secret.json + + // Attempts with current directory references + assertTraversalBlocked("/./../index.html") + assertTraversalBlocked("/././../index.html") + + // Windows-style path attempts (backslashes): + assertTraversalBlocked("/..\\index.html") + assertTraversalBlocked("/..\\..\\index.html") + + // Trailing slashes + assertTraversalBlocked("/../") + assertTraversalBlocked("/../../") + + // Attempts to load files from an absolute path outside the root + assertTraversalBlocked("/" + rootDir + "/../../index.html") + + // Additional edge cases: + + // Double-encoded `..` + assertTraversalBlocked("/%252e%252e/index.html") // double-encoded .. -> ../index.html after double decoding + + // Multiple levels of encoding and traversal + assertTraversalBlocked("/%2e%2e%2F..%2f%2e%2e%2fWINDOWS") // multiple ups and unusual pattern + assertTraversalBlocked("/%2e%2e%2F..%2f%2e%2e%2f%2e%2e/secret") // more complex chain of ../ + + // Null byte attempts + assertTraversalBlocked("/index.html%00.jpg") + assertTraversalBlocked("/%00index.html") + assertTraversalBlocked("/somefolder%00/something") + assertTraversalBlocked("/%00/index.html") + + // Attempts to access known system files + assertTraversalBlocked("/etc/passwd") + assertTraversalBlocked("/etc/") + + // Complex mixed attempts with encoded slashes and dots + assertTraversalBlocked("/..%2F..%2F..%2F..%2Fetc%2Fpasswd") + + // Attempts inside subdirectories with encoded traversal + assertTraversalBlocked("/somefolder/%2e%2e%2findex.html") + assertTraversalBlocked("/somefolder/%2e%2e%2f%2e%2e%2findex.html") + + // Backslash encoded attempts + assertTraversalBlocked("/%5C..%5Cindex.html") +} From d8e225cf37bd9e9bece2facb86285cc4c2ac2fb3 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Tue, 10 Dec 2024 00:35:50 -0500 Subject: [PATCH 4/6] Add separate test for windows --- middleware/static/static.go | 5 +-- middleware/static/static_test.go | 76 ++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/middleware/static/static.go b/middleware/static/static.go index 96830e22cc..9f141372b4 100644 --- a/middleware/static/static.go +++ b/middleware/static/static.go @@ -110,11 +110,8 @@ func New(root string, cfg ...Config) fiber.Handler { return nil } - // Replace backslashes with slashes - safePath := strings.ReplaceAll(string(path), "\\", "/") - // Clean the path and resolve it against the root - cleanPath := filepath.Clean("/" + safePath) + cleanPath := filepath.Clean("/" + utils.UnsafeString(path)) absPath := filepath.Join(absRoot, cleanPath) relPath, err := filepath.Rel(absRoot, absPath) diff --git a/middleware/static/static_test.go b/middleware/static/static_test.go index 71caf1a857..70a08c9fcc 100644 --- a/middleware/static/static_test.go +++ b/middleware/static/static_test.go @@ -921,10 +921,6 @@ func Test_Static_PathTraversal(t *testing.T) { assertTraversalBlocked("/./../index.html") assertTraversalBlocked("/././../index.html") - // Windows-style path attempts (backslashes): - assertTraversalBlocked("/..\\index.html") - assertTraversalBlocked("/..\\..\\index.html") - // Trailing slashes assertTraversalBlocked("/../") assertTraversalBlocked("/../../") @@ -961,3 +957,75 @@ func Test_Static_PathTraversal(t *testing.T) { // Backslash encoded attempts assertTraversalBlocked("/%5C..%5Cindex.html") } + +func Test_Static_PathTraversal_WindowsOnly(t *testing.T) { + // Skip this test if not running on Windows + if runtime.GOOS != "windows" { + t.Skip("Skipping Windows-specific tests") + } + + t.Parallel() + app := fiber.New() + + // Serve only from "../../.github/testdata/fs/css" + rootDir := "../../.github/testdata/fs/css" + app.Get("/*", New(rootDir)) + + // A valid request (relative path without backslash): + validReq := httptest.NewRequest(fiber.MethodGet, "/style.css", nil) + validResp, err := app.Test(validReq) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, 200, validResp.StatusCode, "Status code for valid file on Windows") + body, err := io.ReadAll(validResp.Body) + require.NoError(t, err, "app.Test(req)") + require.Contains(t, string(body), "color") + + // Helper to test blocked responses + assertTraversalBlocked := func(path string) { + req := httptest.NewRequest(fiber.MethodGet, path, nil) + resp, err := app.Test(req) + require.NoError(t, err, "app.Test(req)") + + // We expect a blocked request to return either 400 or 404 + status := resp.StatusCode + require.Containsf(t, []int{400, 404}, status, + "Status code for path traversal %s should be 400 or 404, got %d", path, status) + + // If it's a 404, we expect a "Cannot GET" message + if status == 404 { + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Contains(t, string(body), "Cannot GET", + "Blocked traversal should have a 'Cannot GET' message for %s", path) + } else { + require.Contains(t, string(body), "Are you a hacker?", + "Blocked traversal should have a Cannot GET message for %s", path) + } + } + + // Windows-specific traversal attempts + // Backslashes are treated as directory separators on Windows. + assertTraversalBlocked("/..\\index.html") + assertTraversalBlocked("/..\\..\\index.html") + + // Attempt with a path that might try to reference Windows drives or absolute paths + // Note: These are artificial tests to ensure no drive-letter escapes are allowed. + assertTraversalBlocked("/C:\\Windows\\System32\\cmd.exe") + assertTraversalBlocked("/C:/Windows/System32/cmd.exe") + + // Attempt with UNC-like paths (though unlikely in a web context, good to test) + assertTraversalBlocked("//server\\share\\secret.txt") + + // Attempt using a mixture of forward and backward slashes + assertTraversalBlocked("/..\\..\\/index.html") + + // Attempt that includes a null-byte on Windows + assertTraversalBlocked("/index.html%00.txt") + + // Check behavior on an obviously non-existent and suspicious file + assertTraversalBlocked("/\\this\\path\\does\\not\\exist\\..") + + // Attempts involving relative traversal and current directory reference + assertTraversalBlocked("/.\\../index.html") + assertTraversalBlocked("/./..\\index.html") +} From 42e4a1a1d98a2ff177df325a426033d120a69422 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Tue, 10 Dec 2024 00:48:33 -0500 Subject: [PATCH 5/6] Do not prefix with / --- middleware/static/static.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/middleware/static/static.go b/middleware/static/static.go index 9f141372b4..8e6a0f38e4 100644 --- a/middleware/static/static.go +++ b/middleware/static/static.go @@ -111,7 +111,7 @@ func New(root string, cfg ...Config) fiber.Handler { } // Clean the path and resolve it against the root - cleanPath := filepath.Clean("/" + utils.UnsafeString(path)) + cleanPath := filepath.Clean(utils.UnsafeString(path)) absPath := filepath.Join(absRoot, cleanPath) relPath, err := filepath.Rel(absRoot, absPath) From c65df17d932de6ba7f4195dcb9c7c05c695747ff Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:24:58 -0500 Subject: [PATCH 6/6] Skip linux test if running Windows --- middleware/static/static_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/middleware/static/static_test.go b/middleware/static/static_test.go index 70a08c9fcc..72e65c5bf7 100644 --- a/middleware/static/static_test.go +++ b/middleware/static/static_test.go @@ -852,6 +852,11 @@ func Test_Static_Compress_WithFileSuffixes(t *testing.T) { } func Test_Static_PathTraversal(t *testing.T) { + // Skip this test if running on Windows + if runtime.GOOS == "windows" { + t.Skip("Skipping Windows-specific tests") + } + t.Parallel() app := fiber.New()