diff --git a/fs.go b/fs.go index 076602a..ef38c1c 100644 --- a/fs.go +++ b/fs.go @@ -247,7 +247,7 @@ func normalizeServeFilePath(ctx *RequestCtx, path string) (string, bool) { // Path rewriter is used in FS for translating the current request // to the local filesystem path relative to FS.Root. // -// The returned path must not contain '/../' substrings due to security reasons, +// The returned path must not contain '..' path segments due to security reasons, // since such paths may refer files outside FS.Root. // // The returned path may refer to ctx members. For example, ctx.Path(). @@ -1176,11 +1176,11 @@ func (h *fsHandler) handleRequest(ctx *RequestCtx) { return } if h.pathRewrite != nil { - // There is no need to check for '/../' if path = ctx.Path(), + // There is no need to check rewritten paths if path = ctx.Path(), // since ctx.Path must normalize and sanitize the path. - if n := bytes.Index(path, strSlashDotDotSlash); n >= 0 { - ctx.Logger().Printf("cannot serve path with '/../' at position %d due to security reasons: %q", n, path) + if hasDotDotPathSegment(path) { + ctx.Logger().Printf("cannot serve rewritten path with '..' path segment due to security reasons: %q", path) ctx.Error("Internal Server Error", StatusInternalServerError) return } @@ -1876,6 +1876,27 @@ func stripLeadingSlashes(path []byte, stripSlashes int) []byte { return path } +func hasDotDotPathSegment(path []byte) bool { + segmentStart := 0 + for i := 0; i <= len(path); i++ { + isSeparator := i == len(path) + if i < len(path) { + isSeparator = path[i] == '/' + if filepath.Separator == '\\' && path[i] == '\\' { + isSeparator = true + } + } + if !isSeparator { + continue + } + if i-segmentStart == 2 && path[segmentStart] == '.' && path[segmentStart+1] == '.' { + return true + } + segmentStart = i + 1 + } + return false +} + func fileExtension(path string, compressed bool, compressedFileSuffix string) string { if compressed && strings.HasSuffix(path, compressedFileSuffix) { path = path[:len(path)-len(compressedFileSuffix)] diff --git a/fs_fs_test.go b/fs_fs_test.go index 0ac58bb..9101f4b 100644 --- a/fs_fs_test.go +++ b/fs_fs_test.go @@ -1069,3 +1069,126 @@ func TestFSRootEnforcement(t *testing.T) { }) } } + +func TestHasDotDotPathSegment(t *testing.T) { + t.Parallel() + + testCases := []struct { + path string + want bool + }{ + {path: "", want: false}, + {path: ".", want: false}, + {path: "..", want: true}, + {path: "../secret.txt", want: true}, + {path: "/../secret.txt", want: true}, + {path: "nested/../info", want: true}, + {path: "nested/..", want: true}, + {path: "nested/..hidden/info", want: false}, + {path: "nested..", want: false}, + {path: "/index.html", want: false}, + } + + if filepath.Separator == '\\' { + testCases = append(testCases, + struct { + path string + want bool + }{path: `..\secret.txt`, want: true}, + struct { + path string + want bool + }{path: `nested\..\info`, want: true}, + ) + } + + for _, tc := range testCases { + t.Run(tc.path, func(t *testing.T) { + t.Parallel() + + if got := hasDotDotPathSegment([]byte(tc.path)); got != tc.want { + t.Fatalf("unexpected result for %q: got %v want %v", tc.path, got, tc.want) + } + }) + } +} + +func TestFSPathRewriteRejectsDotDotSegments(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + publicDir := filepath.Join(tmpDir, "public") + if err := os.MkdirAll(publicDir, 0o755); err != nil { + t.Fatalf("cannot create public dir: %v", err) + } + if err := os.WriteFile(filepath.Join(publicDir, "index.html"), []byte("

Public

"), 0o644); err != nil { + t.Fatalf("cannot create public index: %v", err) + } + secretPath := filepath.Join(tmpDir, "secret.txt") + if err := os.WriteFile(secretPath, []byte("TOP_SECRET"), 0o644); err != nil { + t.Fatalf("cannot create secret file: %v", err) + } + + type testCase struct { + name string + pathRewrite PathRewriteFunc + requestURI string + } + + testCases := []testCase{ + { + name: "prefix-stripper-leading-dotdot", + pathRewrite: NewPathPrefixStripper(len("/static/")), + requestURI: "http://localhost/aaaaaaa../secret.txt", + }, + { + name: "custom-leading-dotdot", + pathRewrite: func(ctx *RequestCtx) []byte { + return []byte("../secret.txt") + }, + requestURI: "http://localhost/ignored", + }, + { + name: "custom-trailing-dotdot", + pathRewrite: func(ctx *RequestCtx) []byte { + return []byte("nested/..") + }, + requestURI: "http://localhost/ignored", + }, + { + name: "custom-exact-dotdot", + pathRewrite: func(ctx *RequestCtx) []byte { + return []byte("..") + }, + requestURI: "http://localhost/ignored", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + stop := make(chan struct{}) + defer close(stop) + + fs := &FS{ + Root: publicDir, + AllowEmptyRoot: true, + CleanStop: stop, + PathRewrite: tc.pathRewrite, + } + h := fs.NewRequestHandler() + + var ctx RequestCtx + ctx.Init(&Request{}, nil, TestLogger{t: t}) + ctx.Request.SetRequestURI(tc.requestURI) + + h(&ctx) + + if ctx.Response.StatusCode() != StatusInternalServerError { + t.Fatalf("unexpected status code for %s: %d. Expecting %d", tc.name, ctx.Response.StatusCode(), StatusInternalServerError) + } + if bytes.Contains(ctx.Response.Body(), []byte("TOP_SECRET")) { + t.Fatalf("unexpected secret disclosure for %s", tc.name) + } + }) + } +}