From 3fe2c73dd04f7769a9d9673236cb94b79ac45659 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 30 Dec 2021 04:15:48 -0500 Subject: caddyhttp: Fix `MatchPath` sanitizing (#4499) This is a followup to #4407, in response to a report on the forums: https://caddy.community/t/php-fastcgi-phishing-redirection/14542 Turns out that doing `TrimRight` to remove trailing dots, _before_ cleaning the path, will cause double-dots at the end of the path to not be cleaned away as they should. We should instead remove the dots _after_ cleaning. --- modules/caddyhttp/matchers.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'modules/caddyhttp/matchers.go') diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index 439c407..272c924 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -325,6 +325,11 @@ func (m MatchPath) Match(r *http.Request) bool { lowerPath := strings.ToLower(unescapedPath) + // Clean the path, merges doubled slashes, etc. + // This ensures maliciously crafted requests can't bypass + // the path matcher. See #4407 + lowerPath = path.Clean(lowerPath) + // 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 @@ -332,11 +337,6 @@ 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 + "/" -- cgit v1.2.3