caddyhttp: fix path_regexp (MatchPathRE) Windows backslash bypass (#7858)
Some checks are pending
Tests / test (./cmd/caddy/caddy, ~1.26.0, macos-14, 0, 1.26, mac) (push) Waiting to run
Tests / test (./cmd/caddy/caddy, ~1.26.0, ubuntu-latest, 0, 1.26, linux) (push) Waiting to run
Tests / test (./cmd/caddy/caddy.exe, ~1.26.0, windows-latest, True, 1.26, windows) (push) Waiting to run
Tests / test (s390x on IBM Z) (push) Waiting to run
Tests / goreleaser-check (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, aix) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, darwin) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, dragonfly) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, freebsd) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, illumos) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, linux) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, netbsd) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, openbsd) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, solaris) (push) Waiting to run
Cross-Build / build (~1.26.0, 1.26, windows) (push) Waiting to run
Lint / lint (push) Waiting to run
Lint / lint-1 (push) Waiting to run
Lint / lint-2 (push) Waiting to run
Lint / govulncheck (push) Waiting to run
Lint / dependency-review (push) Waiting to run
OpenSSF Scorecard supply-chain security / Scorecard analysis (push) Waiting to run

* caddyhttp: normalize Windows path in path_regexp matcher, shared with path matcher

Apply the same Windows path normalization to MatchPathRE that MatchPath already had, and factor it into a shared normalizeWindowsPath helper so both matchers use one implementation.

Also strip trailing dots and spaces per path component (not only at the end of the whole path), matching how Windows resolves paths; this fixes the same gap in the path matcher too. Adds regression tests for both matchers.

See #5613.

* caddyhttp: normalize trailing dots/spaces in escaped-path branch too

The MatchPath escaped-path branch (taken when a matcher pattern contains
'%') only folded backslash separators via windowsEscapedPathSeparatorRepl;
it skipped the per-component trailing dot/space normalization now applied
on the decoded branch. On Windows this left a bypass: a matcher such as
`path /private%2f*` could be evaded by GET /private.%5csecret.txt, since
`private.` and `private` resolve to the same directory on NTFS.

Add normalizeWindowsEscapedPath, which trims trailing dots and spaces —
literal ("." / " ") and percent-encoded ("%2e" / "%20") — from every
component in raw/escaped space, splitting on both '/' and encoded '%2f'
separators while preserving them, and leaving "." / ".." for CleanPath.
Regression tests cover the literal and percent-encoded variants against a
'%'-containing matcher.

---------

Co-authored-by: thientd <thien.taduc@ninhthanh.com>
This commit is contained in:
Ta Duc Thien 2026-07-11 05:37:44 +07:00 committed by GitHub
parent 1830809afe
commit c6180a0852
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 198 additions and 12 deletions

View file

@ -433,16 +433,7 @@ func (m MatchPath) MatchWithError(r *http.Request) (bool, error) {
// related to differences between operating systems, applications,
// etc; if case-sensitive matching is needed, the regex matcher
// can be used instead.
reqPath := strings.ToLower(r.URL.Path)
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, ". ")
}
reqPath := normalizeWindowsPath(strings.ToLower(r.URL.Path))
repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
@ -482,6 +473,7 @@ func (m MatchPath) MatchWithError(r *http.Request) (bool, error) {
if runtime.GOOS == "windows" {
escapedPath = windowsEscapedPathSeparatorRepl.Replace(escapedPath)
matchPattern = windowsEscapedPathSeparatorRepl.Replace(matchPattern)
escapedPath = normalizeWindowsEscapedPath(escapedPath)
}
reqPathForPattern := CleanPath(escapedPath, mergeSlashes)
if m.matchPatternWithEscapeSequence(reqPathForPattern, matchPattern) {
@ -670,6 +662,98 @@ var windowsEscapedPathSeparatorRepl = strings.NewReplacer(
"%5C", "%2f",
)
// normalizeWindowsPath rewrites a decoded request path the way the Windows
// filesystem resolves it, so that path matchers (both `path` and `path_regexp`)
// cannot be bypassed on Windows: backslashes are treated as path separators,
// and trailing dots and spaces are ignored on EVERY path component (not only at
// the end of the whole path — e.g. a guard on `/private/` must also cover
// `/private./secret`). Path navigation segments ("." and "..") are left intact
// for cleanPath to resolve. It is a no-op on non-Windows systems. See #5613.
func normalizeWindowsPath(p string) string {
if runtime.GOOS != "windows" {
return p
}
p = strings.ReplaceAll(p, `\`, "/")
segments := strings.Split(p, "/")
for i, s := range segments {
if s == "." || s == ".." {
continue
}
segments[i] = strings.TrimRight(s, ". ")
}
return strings.Join(segments, "/")
}
// normalizeWindowsEscapedPath applies the same Windows normalization as
// normalizeWindowsPath, but in raw/escaped space (used when a matcher pattern
// contains '%'), where path separators may be percent-encoded ("%2f") and a
// component's trailing dots and spaces may be literal ("." / " ") or
// percent-encoded ("%2e" / "%20"). Without this, a '%'-containing matcher such
// as `/private%2f*` is bypassed on Windows by e.g. GET /private.%5csecret.txt,
// because only backslash separators — not the per-component trailing dots and
// spaces — were folded. The caller must have already folded backslash
// separators to "%2f" via windowsEscapedPathSeparatorRepl. Navigation segments
// ("." and "..") are left intact for CleanPath to resolve. Only the request
// path is normalized, not the matcher pattern (mirroring normalizeWindowsPath).
// It is a no-op on non-Windows systems. See #5613.
func normalizeWindowsEscapedPath(p string) string {
if runtime.GOOS != "windows" {
return p
}
var sb strings.Builder
sb.Grow(len(p))
// walk the path, splitting on separators that are meaningful on Windows:
// the literal '/' and the encoded '%2f'/'%2F' (backslashes were already
// folded to "%2f" by the caller). Trim each component, preserving the
// separators exactly so escaped-space comparison still lines up.
start := 0
flush := func(end int) {
seg := p[start:end]
if seg != "." && seg != ".." {
seg = trimWindowsEscapedTrailingDotSpace(seg)
}
sb.WriteString(seg)
}
for i := 0; i < len(p); {
switch {
case p[i] == '/':
flush(i)
sb.WriteByte('/')
i++
start = i
case p[i] == '%' && i+3 <= len(p) && p[i+1] == '2' && (p[i+2] == 'f' || p[i+2] == 'F'):
flush(i)
sb.WriteString(p[i : i+3])
i += 3
start = i
default:
i++
}
}
flush(len(p))
return sb.String()
}
// trimWindowsEscapedTrailingDotSpace strips trailing dots and spaces — whether
// literal ("." / " ") or percent-encoded ("%2e"/"%2E" / "%20") — from a single
// escaped-space path component, matching how Windows ignores them.
func trimWindowsEscapedTrailingDotSpace(s string) string {
for len(s) > 0 {
if c := s[len(s)-1]; c == '.' || c == ' ' {
s = s[:len(s)-1]
continue
}
if len(s) >= 3 {
if tail := s[len(s)-3:]; strings.EqualFold(tail, "%2e") || tail == "%20" {
s = s[:len(s)-3]
continue
}
}
return s
}
return s
}
// CELLibrary produces options that expose this matcher for use in CEL
// expression matchers.
//
@ -730,8 +814,10 @@ func (m MatchPathRE) MatchWithError(r *http.Request) (bool, error) {
// Clean the path, merges doubled slashes, etc.
// This ensures maliciously crafted requests can't bypass
// the path matcher. See #4407
cleanedPath := cleanPath(r.URL.Path)
// the path matcher. See #4407 (path cleaning) and #5613
// (Windows backslash / trailing dot-and-space normalization,
// shared with the path matcher).
cleanedPath := cleanPath(normalizeWindowsPath(r.URL.Path))
return m.MatchRegexp.Match(cleanedPath, repl), nil
}

View file

@ -509,6 +509,34 @@ func TestPathMatcherWindows(t *testing.T) {
requestTarget: `/private%5csecret.txt`,
match: MatchPath{"/private%5c%*"},
},
{
name: "trailing dot on a middle path component",
path: "/private./secret.txt",
match: MatchPath{"/private/*"},
},
{
name: "trailing space on a middle path component",
path: "/private /secret.txt",
match: MatchPath{"/private/*"},
},
{
// escaped-space matcher (pattern contains '%'): the trailing
// dot/space normalization must also apply on the escaped-path
// branch, otherwise /private.%5csecret.txt bypasses /private%2f*.
name: "trailing dot before encoded backslash, escaped-space matcher",
requestTarget: `/private.%5csecret.txt`,
match: MatchPath{"/private%2f*"},
},
{
name: "encoded trailing dot before encoded backslash, escaped-space matcher",
requestTarget: `/private%2e%5csecret.txt`,
match: MatchPath{"/private%2f*"},
},
{
name: "encoded trailing space before encoded backslash, escaped-space matcher",
requestTarget: `/private%20%5csecret.txt`,
match: MatchPath{"/private%2f*"},
},
} {
t.Run(tc.name, func(t *testing.T) {
u := &url.URL{Path: tc.path}
@ -534,6 +562,78 @@ func TestPathMatcherWindows(t *testing.T) {
}
}
func TestPathREMatcherWindows(t *testing.T) {
// Windows treats backslashes as path separators and ignores trailing
// dots and spaces per path component, so the path_regexp matcher must
// normalize them the same way the path matcher does (see #5613);
// otherwise a guard such as `path_regexp ^/private/` is bypassed by e.g.
// GET /private\secret.txt (or %5c) or GET /private./secret.txt.
if runtime.GOOS != "windows" {
return
}
repl := caddy.NewReplacer()
for _, tc := range []struct {
name string
path string
requestTarget string
match MatchPathRE
}{
{
name: "literal backslash path separator",
path: `/private\secret.txt`,
match: MatchPathRE{MatchRegexp{Pattern: "^/private/"}},
},
{
name: "encoded backslash path separator",
requestTarget: `/private%5csecret.txt`,
match: MatchPathRE{MatchRegexp{Pattern: "^/private/"}},
},
{
name: "uppercase encoded backslash path separator",
requestTarget: `/private%5Csecret.txt`,
match: MatchPathRE{MatchRegexp{Pattern: "^/private/"}},
},
{
name: "trailing dot on a middle path component",
path: `/private./secret.txt`,
match: MatchPathRE{MatchRegexp{Pattern: "^/private/"}},
},
{
name: "trailing space on a middle path component",
path: `/private /secret.txt`,
match: MatchPathRE{MatchRegexp{Pattern: "^/private/"}},
},
} {
t.Run(tc.name, func(t *testing.T) {
if err := tc.match.Provision(caddy.Context{}); err != nil {
t.Fatalf("Provisioning: %v", err)
}
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.Pattern)
}
})
}
}
func TestPathREMatcher(t *testing.T) {
for i, tc := range []struct {
match MatchPathRE