summaryrefslogtreecommitdiff
path: root/modules/caddyhttp
diff options
context:
space:
mode:
authorFrancis Lavoie <lavofr@gmail.com>2021-11-08 15:45:03 -0500
committerGitHub <noreply@github.com>2021-11-08 13:45:03 -0700
commite7457b43e4703080ae8713ada798ce3e20b83690 (patch)
treea02db22f8e7ba9c15a3aae5c2551c76ddf7e0434 /modules/caddyhttp
parentf376a38b254a4fa469df10914180c2ebab3e707e (diff)
caddyhttp: Sanitize the path before evaluating path matchers (#4407)
Diffstat (limited to 'modules/caddyhttp')
-rw-r--r--modules/caddyhttp/matchers.go42
-rw-r--r--modules/caddyhttp/matchers_test.go34
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,