diff options
author | Matthew Holt <[email protected]> | 2019-12-17 10:14:04 -0700 |
---|---|---|
committer | Matthew Holt <[email protected]> | 2019-12-17 10:14:04 -0700 |
commit | dae4913fe35ff3f8a97383061ea8d44c1e98279e (patch) | |
tree | 5d6cd9453b225fbdb3fc86d815d4d7560bca6ea5 | |
parent | 6455efa5d327377b41e708a063d1b7a71a131165 (diff) | |
download | caddy-dae4913fe35ff3f8a97383061ea8d44c1e98279e.tar.gz caddy-dae4913fe35ff3f8a97383061ea8d44c1e98279e.zip |
http: Patch path matcher to ignore dots and spaces (#2917)
(Try saying "patch path match" ten times fast)
-rw-r--r-- | modules/caddyhttp/matchers.go | 8 | ||||
-rw-r--r-- | modules/caddyhttp/matchers_test.go | 12 | ||||
-rw-r--r-- | modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go | 3 | ||||
-rw-r--r-- | modules/caddyhttp/routes.go | 2 |
4 files changed, 23 insertions, 2 deletions
diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index eaa27f0d9..6d1728d6f 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -165,6 +165,14 @@ 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) + + // 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 + lowerPath = strings.TrimRight(lowerPath, ". ") + for _, matchPath := range m { // special case: first character is equals sign, // treat it as an exact match diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go index f5ec034f8..34a1647c0 100644 --- a/modules/caddyhttp/matchers_test.go +++ b/modules/caddyhttp/matchers_test.go @@ -247,6 +247,18 @@ func TestPathMatcher(t *testing.T) { } } +func TestPathMatcherWindows(t *testing.T) { + // only Windows has this bug where it will ignore + // trailing dots and spaces in a filename, but we + // test for it on all platforms to be more consistent + match := MatchPath{"*.php"} + req := &http.Request{URL: &url.URL{Path: "/index.php . . .."}} + matched := match.Match(req) + if !matched { + t.Errorf("Expected to match; should ignore trailing dots and spaces") + } +} + func TestPathREMatcher(t *testing.T) { for i, tc := range []struct { match MatchPathRE diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index 699f6808a..531704849 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -115,7 +115,8 @@ func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // php_fastcgi /subpath localhost:7777 // // then the resulting routes are wrapped in a subroute that uses the -// user's matcher as a prerequisite to enter the subroute. +// user's matcher as a prerequisite to enter the subroute. In other +// words, the directive's matcher is necessary, but not sufficient. func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) { if !h.Next() { return nil, h.ArgErr() diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go index 0dce990d5..9b2f84922 100644 --- a/modules/caddyhttp/routes.go +++ b/modules/caddyhttp/routes.go @@ -115,7 +115,7 @@ func (routes RouteList) Provision(ctx caddy.Context) error { // matchers matchersIface, err := ctx.LoadModule(&route, "MatcherSetsRaw") if err != nil { - return fmt.Errorf("loadng matchers in route %d: %v", i, err) + return fmt.Errorf("loading matchers in route %d: %v", i, err) } err = routes[i].MatcherSets.FromInterface(matchersIface) if err != nil { |