http: Patch path matcher to ignore dots and spaces (#2917)

(Try saying "patch path match" ten times fast)

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 {