caddyhttp: preserve invalid UTF-8 bytes throughout path matching

Lowercasing and slash-preserving path cleaning rewrote invalid UTF-8
bytes as U+FFFD, so the path matcher could evaluate a different path
than the file server opens. Preserve the original bytes in both, keeping
case-insensitive matching for valid Unicode.

Addresses scan report 18c02f60a5b43529.

Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Matthew HoltandCopilot committed 2026-11-11 11:06:40 -07:00
1 parent c3fab6576f
commit ebfc03a7b6
4 files changed
+175 -20

No files matched your search

+37 -15
View File
@@ -275,28 +275,50 @@ func SanitizedPathJoin(root, reqPath string) string {
// CleanPath cleans path p according to path.Clean(), but only
// merges repeated slashes if collapseSlashes is true, and always
// preserves trailing slashes.
// preserves trailing slashes. Path bytes, including invalid UTF-8, are preserved.
func CleanPath(p string, collapseSlashes bool) string {
if collapseSlashes {
if collapseSlashes || !strings.Contains(p, "//") {
return cleanPath(p)
}
// insert an invalid/impossible URI character into each two consecutive
// slashes to expand empty path segments; then clean the path as usual,
// and then remove the remaining temporary characters.
const tmpCh = 0xff
var sb strings.Builder
sb.Grow(len(p) + 1)
for i, ch := range p {
if ch == '/' && i > 0 && p[i-1] == '/' {
sb.WriteByte(tmpCh)
rooted := strings.HasPrefix(p, "/")
trailing := strings.HasSuffix(p, "/")
segments := strings.Split(p, "/")
if rooted {
segments = segments[1:]
}
if trailing && len(segments) > 0 {
segments = segments[:len(segments)-1]
}
cleaned := segments[:0]
for _, segment := range segments {
switch segment {
case ".":
case "..":
if len(cleaned) > 0 && cleaned[len(cleaned)-1] != ".." {
cleaned = cleaned[:len(cleaned)-1]
} else if !rooted {
cleaned = append(cleaned, segment)
}
default:
cleaned = append(cleaned, segment)
}
sb.WriteRune(ch)
}
result := strings.Join(cleaned, "/")
if rooted {
result = "/" + result
}
if trailing && len(cleaned) > 0 {
result += "/"
}
if result == "" && len(cleaned) == 0 {
result = "."
if trailing {
result += "/"
}
}
halfCleaned := cleanPath(sb.String())
halfCleaned = strings.ReplaceAll(halfCleaned, string([]byte{tmpCh}), "")
return halfCleaned
return result
}
// cleanPath does path.Clean(p) but preserves any trailing slash.
+60
View File
@@ -4,6 +4,7 @@ import (
"net/url"
"path/filepath"
"runtime"
"strings"
"testing"
)
@@ -147,6 +148,65 @@ func TestSanitizedPathJoin(t *testing.T) {
}
}
func TestCleanPathPreservesBytes(t *testing.T) {
for b := 0; b < 256; b++ {
if b == '/' {
continue
}
input := "/public//byte" + string([]byte{byte(b)}) + "/file"
if got := CleanPath(input, false); got != input {
t.Errorf("CleanPath(%q, false) = %q; want unchanged bytes", input, got)
}
}
for _, tc := range []struct {
input, want string
}{
{"/\xff//a/../\xfe/", "/\xff//\xfe/"},
{"/Äpfel//./�/../\xff", "/Äpfel//\xff"},
{"//", "//"},
{"///", "///"},
{"//.", "/"},
{"//..", "/"},
{"a/..//b", "/b"},
{".//", "/"},
{"a//../../..", ".."},
{"", "."},
{"./", "./"},
{"../", "../"},
} {
if got := CleanPath(tc.input, false); got != tc.want {
t.Errorf("CleanPath(%q, false) = %q; want %q", tc.input, got, tc.want)
}
}
}
func TestCleanPathSlashPreservingSemantics(t *testing.T) {
var check func([]string, int)
check = func(segments []string, depth int) {
p := strings.Join(segments, "/")
for _, input := range []string{p, "/" + p, p + "/", "/" + p + "/"} {
var marked strings.Builder
for i := 0; i < len(input); i++ {
if input[i] == '/' && i > 0 && input[i-1] == '/' {
marked.WriteByte(0)
}
marked.WriteByte(input[i])
}
want := strings.ReplaceAll(cleanPath(marked.String()), "\x00", "")
if got := CleanPath(input, false); got != want {
t.Errorf("CleanPath(%q, false) = %q; want %q", input, got, want)
}
}
if depth == 0 {
return
}
for _, segment := range []string{"", "a", ".", "..", "Ä", "%2f", "a\xff"} {
check(append(segments, segment), depth-1)
}
}
check(nil, 4)
}
func TestCleanPath(t *testing.T) {
for i, tc := range []struct {
input string
+25 -5
View File
@@ -29,6 +29,7 @@ import (
"sort"
"strconv"
"strings"
"unicode"
"unicode/utf8"
"cel.dev/cel-go/cel"
@@ -419,7 +420,7 @@ func (m MatchPath) Provision(_ caddy.Context) error {
break
}
if !strings.Contains(m[i], "{") {
m[i] = strings.ToLower(m[i])
m[i] = lowerPath(m[i])
}
}
return nil
@@ -439,13 +440,13 @@ func (m MatchPath) MatchWithError(r *http.Request) (bool, error) {
// related to differences between operating systems, applications,
// etc; if case-sensitive matching is needed, the regex matcher
// can be used instead.
reqPath := normalizeWindowsPath(strings.ToLower(r.URL.Path))
reqPath := normalizeWindowsPath(lowerPath(r.URL.Path))
repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
for _, matchPattern := range m {
if strings.Contains(matchPattern, "{") {
matchPattern = strings.ToLower(repl.ReplaceAll(matchPattern, ""))
matchPattern = lowerPath(repl.ReplaceAll(matchPattern, ""))
}
// special case: whole path is wildcard; this is unnecessary
@@ -543,7 +544,7 @@ func (m MatchPath) MatchWithError(r *http.Request) (bool, error) {
}
func (MatchPath) matchPatternWithEscapeSequence(escapedPath, matchPath string) bool {
escapedPath = strings.ToLower(escapedPath)
escapedPath = lowerPath(escapedPath)
// We would just compare the pattern against r.URL.Path,
// but the pattern contains %, indicating that we should
// compare at least some part of the path in raw/escaped
@@ -660,10 +661,29 @@ func (MatchPath) matchPatternWithEscapeSequence(escapedPath, matchPath string) b
matchPath = strings.ReplaceAll(matchPath, "%*", "*")
// ignore error here because we can't handle it anyway
matches, _ := path.Match(matchPath, strings.ToLower(sb.String()))
matches, _ := path.Match(matchPath, lowerPath(sb.String()))
return matches
}
// lowerPath lowercases Unicode text without replacing invalid UTF-8 path bytes.
func lowerPath(p string) string {
if utf8.ValidString(p) {
return strings.ToLower(p)
}
var sb strings.Builder
sb.Grow(len(p))
for len(p) > 0 {
ch, size := utf8.DecodeRuneInString(p)
if ch == utf8.RuneError && size == 1 {
sb.WriteByte(p[0])
} else {
sb.WriteRune(unicode.ToLower(ch))
}
p = p[size:]
}
return sb.String()
}
// windowsEscapedPathSeparatorRepl normalizes Windows backslash separators
// while preserving escaped-path matching semantics.
var windowsEscapedPathSeparatorRepl = strings.NewReplacer(
+53
View File
@@ -553,6 +553,59 @@ func TestEscapedPathWildcardCursor(t *testing.T) {
}
}
func TestLowerPathPreservesInvalidUTF8(t *testing.T) {
for b := 128; b < 256; b++ {
pathByte := string([]byte{byte(b)})
input := "Ä" + pathByte + "Σ"
want := "ä" + pathByte + "σ"
if got := lowerPath(input); got != want {
t.Errorf("lowerPath(%q) = %q; want %q", input, got, want)
}
}
}
func TestPathMatcherPreservesInvalidUTF8(t *testing.T) {
for _, tc := range []struct {
pattern, target string
want bool
}{
{"/public//�", "/public/%ff", false},
{"/public//�", "/public//%EF%BF%BD", true},
{"/public//�", "/public//%ff", false},
{"/public/�", "/public/%ff", false},
{"/public//�", "/public//%80", false},
{"/public//�", "/public//%c0%af", false},
{"/public//�", "/public//%ef%bf", false},
{"/public//�", "/public//%ed%a0%80", false},
{"/public//�%2fend", "/public//%ff%2fend", false},
{"/public//*�%2f", "/public//%ff%2f", false},
{"/public//�%2fend", "/public//%ef%bf%bd%2fend", true},
{"/ÄPFEL//Σ/İ", "/%c3%a4pfel//%cf%83/i", true},
{"/Ä*%2f", "/%C3%84pfel%2f", true},
{"/Ä//\xff", "/%c3%a4//%ff", true},
{"/Ä//\xff", "/%c3%a4//%ef%bf%bd", false},
} {
t.Run(tc.pattern+" "+tc.target, func(t *testing.T) {
for _, dynamic := range []bool{false, true} {
m := MatchPath{tc.pattern}
r := httptest.NewRequest(http.MethodGet, tc.target, nil)
repl := NewTestReplacer(r)
if dynamic {
m[0] = "{test.path}"
repl.Set("test.path", tc.pattern)
}
if err := m.Provision(caddy.Context{}); err != nil {
t.Fatal(err)
}
got, err := m.MatchWithError(r)
if err != nil || got != tc.want {
t.Errorf("dynamic=%t: match = %t, %v; want %t", dynamic, got, err, tc.want)
}
}
})
}
}
func TestPathMatcherWindows(t *testing.T) {
// only Windows has this bug where it will ignore
// trailing dots and spaces in a filename