diff options
| -rw-r--r-- | modules/caddyhttp/matchers.go | 42 | ||||
| -rw-r--r-- | modules/caddyhttp/matchers_test.go | 34 |
2 files changed, 72 insertions, 4 deletions
diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index 073f82f..439c407 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -21,6 +21,7 @@ import ( "net/http" "net/textproto" "net/url" + "path" "path/filepath" "regexp" "sort" @@ -314,7 +315,15 @@ func (m MatchPath) Provision(_ caddy.Context) error { // Match returns true if r matches m. func (m MatchPath) Match(r *http.Request) bool { - lowerPath := strings.ToLower(r.URL.Path) + // PathUnescape returns an error if the escapes aren't + // well-formed, meaning the count % matches the RFC. + // Return early if the escape is improper. + unescapedPath, err := url.PathUnescape(r.URL.Path) + if err != nil { + return false + } + + lowerPath := strings.ToLower(unescapedPath) // see #2917; Windows ignores trailing dots and spaces // when accessing files (sigh), potentially causing a @@ -323,6 +332,16 @@ func (m MatchPath) Match(r *http.Request) bool { // being matched by *.php to be treated as PHP scripts lowerPath = strings.TrimRight(lowerPath, ". ") + // Clean the path, merges doubled slashes, etc. + // This ensures maliciously crafted requests can't bypass + // the path matcher. See #4407 + lowerPath = path.Clean(lowerPath) + + // Cleaning may remove the trailing slash, but we want to keep it + if lowerPath != "/" && strings.HasSuffix(r.URL.Path, "/") { + lowerPath = lowerPath + "/" + } + repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) for _, matchPath := range m { @@ -396,7 +415,26 @@ func (MatchPathRE) CaddyModule() caddy.ModuleInfo { // Match returns true if r matches m. func (m MatchPathRE) Match(r *http.Request) bool { repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - return m.MatchRegexp.Match(r.URL.Path, repl) + + // PathUnescape returns an error if the escapes aren't + // well-formed, meaning the count % matches the RFC. + // Return early if the escape is improper. + unescapedPath, err := url.PathUnescape(r.URL.Path) + if err != nil { + return false + } + + // Clean the path, merges doubled slashes, etc. + // This ensures maliciously crafted requests can't bypass + // the path matcher. See #4407 + cleanedPath := path.Clean(unescapedPath) + + // Cleaning may remove the trailing slash, but we want to keep it + if cleanedPath != "/" && strings.HasSuffix(r.URL.Path, "/") { + cleanedPath = cleanedPath + "/" + } + + return m.MatchRegexp.Match(cleanedPath, repl) } // CaddyModule returns the Caddy module information. diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go index 2ec7039..f394921 100644 --- a/modules/caddyhttp/matchers_test.go +++ b/modules/caddyhttp/matchers_test.go @@ -258,6 +258,21 @@ func TestPathMatcher(t *testing.T) { expect: true, }, { + match: MatchPath{"/foo*"}, + input: "//foo/bar", + expect: true, + }, + { + match: MatchPath{"/foo*"}, + input: "//foo", + expect: true, + }, + { + match: MatchPath{"/foo*"}, + input: "/%2F/foo", + expect: true, + }, + { match: MatchPath{"*"}, input: "/", expect: true, @@ -326,16 +341,31 @@ func TestPathREMatcher(t *testing.T) { expect: true, }, { - match: MatchPathRE{MatchRegexp{Pattern: "/foo"}}, + match: MatchPathRE{MatchRegexp{Pattern: "^/foo"}}, input: "/foo", expect: true, }, { - match: MatchPathRE{MatchRegexp{Pattern: "/foo"}}, + match: MatchPathRE{MatchRegexp{Pattern: "^/foo"}}, input: "/foo/", expect: true, }, { + match: MatchPathRE{MatchRegexp{Pattern: "^/foo"}}, + input: "//foo", + expect: true, + }, + { + match: MatchPathRE{MatchRegexp{Pattern: "^/foo"}}, + input: "//foo/", + expect: true, + }, + { + match: MatchPathRE{MatchRegexp{Pattern: "^/foo"}}, + input: "/%2F/foo/", + expect: true, + }, + { match: MatchPathRE{MatchRegexp{Pattern: "/bar"}}, input: "/foo/", expect: false, |