From dae4913fe35ff3f8a97383061ea8d44c1e98279e Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Tue, 17 Dec 2019 10:14:04 -0700 Subject: http: Patch path matcher to ignore dots and spaces (#2917) (Try saying "patch path match" ten times fast) --- modules/caddyhttp/matchers.go | 8 ++++++++ modules/caddyhttp/matchers_test.go | 12 ++++++++++++ modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go | 3 ++- modules/caddyhttp/routes.go | 2 +- 4 files changed, 23 insertions(+), 2 deletions(-) (limited to 'modules/caddyhttp') diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index eaa27f0..6d1728d 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 f5ec034..34a1647 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 699f680..5317048 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 0dce990..9b2f849 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 { -- cgit v1.2.3