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.
This commit is contained in:
Francis Lavoie 2021-12-30 04:15:48 -05:00 committed by GitHub
parent 5333c3528b
commit 3fe2c73dd0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -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 + "/"