caddyhttp: Sanitize the path before evaluating path matchers (#4407)

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,