diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index f179b9c11..9f84a90da 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -435,12 +435,12 @@ func (m MatchPath) MatchWithError(r *http.Request) (bool, error) { // can be used instead. reqPath := strings.ToLower(r.URL.Path) - // See #2917; Windows ignores trailing dots and spaces - // when accessing files (sigh), potentially causing a - // security risk (cry) if PHP files end up being served - // as static files, exposing the source code, instead of - // being matched by *.php to be treated as PHP scripts. if runtime.GOOS == "windows" { // issue #5613 + // Windows treats backslashes as path separators and + // ignores trailing dots and spaces when accessing files + // (sigh), potentially causing a security risk (cry) if + // protected files are not matched as intended. + reqPath = strings.ReplaceAll(reqPath, `\`, "/") reqPath = strings.TrimRight(reqPath, ". ") } @@ -478,7 +478,12 @@ func (m MatchPath) MatchWithError(r *http.Request) (bool, error) { // the intent is to compare that part of the path in raw/escaped // space; i.e. "%40"=="%40", not "@", and "%2F"=="%2F", not "/" if strings.Contains(matchPattern, "%") { - reqPathForPattern := CleanPath(r.URL.EscapedPath(), mergeSlashes) + escapedPath := r.URL.EscapedPath() + if runtime.GOOS == "windows" { + escapedPath = windowsEscapedPathSeparatorRepl.Replace(escapedPath) + matchPattern = windowsEscapedPathSeparatorRepl.Replace(matchPattern) + } + reqPathForPattern := CleanPath(escapedPath, mergeSlashes) if m.matchPatternWithEscapeSequence(reqPathForPattern, matchPattern) { return true, nil } @@ -643,6 +648,14 @@ func (MatchPath) matchPatternWithEscapeSequence(escapedPath, matchPath string) b return matches } +// windowsEscapedPathSeparatorRepl normalizes Windows backslash separators +// while preserving escaped-path matching semantics. +var windowsEscapedPathSeparatorRepl = strings.NewReplacer( + `\`, "%2f", + "%5c", "%2f", + "%5C", "%2f", +) + // CELLibrary produces options that expose this matcher for use in CEL // expression matchers. // diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go index c3d8c405e..c0f02d23c 100644 --- a/modules/caddyhttp/matchers_test.go +++ b/modules/caddyhttp/matchers_test.go @@ -461,18 +461,61 @@ func TestPathMatcherWindows(t *testing.T) { return } - req := &http.Request{URL: &url.URL{Path: "/index.php . . .."}} repl := caddy.NewReplacer() - ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl) - req = req.WithContext(ctx) - match := MatchPath{"*.php"} - matched, err := match.MatchWithError(req) - if err != nil { - t.Errorf("Expected no error, but got: %v", err) - } - if !matched { - t.Errorf("Expected to match; should ignore trailing dots and spaces") + for _, tc := range []struct { + name string + path string + requestTarget string + match MatchPath + }{ + { + name: "trailing dots and spaces", + path: "/index.php . . ..", + match: MatchPath{"*.php"}, + }, + { + name: "encoded backslash path separator", + requestTarget: `/private%5csecret.txt`, + match: MatchPath{"/private/*"}, + }, + { + name: "encoded backslash path separator with escaped wildcard", + requestTarget: `/private%5csecret.txt`, + match: MatchPath{"/private/%*"}, + }, + { + name: "uppercase encoded backslash path separator with escaped wildcard", + requestTarget: `/private%5Csecret.txt`, + match: MatchPath{"/private/%*"}, + }, + { + name: "encoded backslash in escaped pattern", + requestTarget: `/private%5csecret.txt`, + match: MatchPath{"/private%5c%*"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + u := &url.URL{Path: tc.path} + if tc.requestTarget != "" { + var err error + u, err = url.ParseRequestURI(tc.requestTarget) + if err != nil { + t.Fatalf("Parsing request target: %v", err) + } + } + req := &http.Request{URL: u} + ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl) + req = req.WithContext(ctx) + + matched, err := tc.match.MatchWithError(req) + if err != nil { + t.Errorf("Expected no error, but got: %v", err) + } + if !matched { + t.Errorf("Expected %q to match %v", req.URL.Path, tc.match) + } + }) } }