fs: reject '..' path segments in rewritten paths (#2173)

Rewritten FS paths were only checked for the "/../" substring, which
allowed leading "../" values to bypass the traversal guard.

Reject any rewritten path containing a ".." path segment before joining
it with FS.Root. This closes the PathRewrite/NewPathPrefixStripper escape
in the default OS-backed handler and keeps rewritten paths within the
intended static root.

This vulnerability was discovered and reported by bugbunny.ai
This commit is contained in:
Erik Dubbelboer
2026-04-05 14:03:25 +09:00
committed by GitHub
parent a95a1ad11c
commit 267e740f56
2 changed files with 148 additions and 4 deletions
+25 -4
View File
@@ -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)]
+123
View File
@@ -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("<h1>Public</h1>"), 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)
}
})
}
}