diff --git a/cgi.go b/cgi.go index 9fcbd8fe46..68adcde64e 100644 --- a/cgi.go +++ b/cgi.go @@ -22,7 +22,6 @@ import ( "path/filepath" "runtime" "strings" - "unicode/utf8" "unsafe" "github.com/dunglas/frankenphp/internal/phpheaders" @@ -233,66 +232,55 @@ func splitCgiPath(fc *frankenPHPContext) { // example: if splitPath is [".php"] // "/path/to/script.php/some/path": ("/path/to/script.php", "/some/path") // -// Matching is strictly ASCII case-insensitive. Bytes >= utf8.RuneSelf in path -// never match any split entry: split strings are validated ASCII-only and -// lower-cased in WithRequestSplitPath, so any Unicode equivalence (e.g. -// fullwidth or mathematical letters folding to ASCII) would let an attacker -// upload a file whose name contains such code points and have it served as -// PHP. See GHSA-3g8v-8r37-cgjm and GHSA-v4h7-cj44-8fc8. +// Matching is ASCII case-insensitive and requires lowercase ASCII markers from normalizeSplitPath. func splitPos(path string, splitPath []string) int { if len(splitPath) == 0 { return 0 } - - pathLen := len(path) - for _, split := range splitPath { splitLen := len(split) - if splitLen == 0 || splitLen > pathLen { + if splitLen == 0 || splitLen > len(path) { continue } - - for i := 0; i <= pathLen-splitLen; i++ { - match := true - for j := range splitLen { - c := path[i+j] - if c >= utf8.RuneSelf { - match = false - - break - } - - if 'A' <= c && c <= 'Z' { - c += 'a' - 'A' - } - - if c != split[j] { - match = false - - break - } + first, mask := split[0], foldMask(split[0]) + last := len(path) - splitLen + i := 0 + if mask == 0 { + if i = strings.IndexByte(path[:last+1], first); i < 0 { + continue } - - if match { - pos := i + splitLen - // the split must end the path or a whole path segment: without - // this, ".php" embedded in a filename ("/a.php.txt/b.php") - // would split inside that filename and run a script the router - // never matched. Give up on this split string entirely rather - // than looking for a later occurrence of it, so the result - // stays identical to Caddy's file matcher (firstSplit). - if pos != pathLen && path[pos] != '/' { - break - } - + } + for ; i <= last; i++ { + if path[i]|mask != first || !hasFoldedPrefix(path[i+1:i+splitLen], split[1:]) { + continue + } + if pos := i + splitLen; pos == len(path) || path[pos] == '/' { return pos } + // Reject an embedded match (e.g. /a.php.txt/b.php) without trying later occurrences. + break } } - return -1 } +// foldMask lets c|mask == s match both cases of a lowercase ASCII letter. +func foldMask(s byte) byte { + if 'a' <= s && s <= 'z' { + return 0x20 + } + return 0 +} + +func hasFoldedPrefix(p, split string) bool { + for j := 0; j < len(split); j++ { + if s := split[j]; p[j]|foldMask(s) != s { + return false + } + } + return true +} + // go_update_request_info updates the sapi_request_info struct // See: https://github.com/php/php-src/blob/345e04b619c3bc11ea17ee02cdecad6ae8ce5891/main/SAPI.h#L72 // diff --git a/cgi_test.go b/cgi_test.go index 81107c55c2..42f9fa6d84 100644 --- a/cgi_test.go +++ b/cgi_test.go @@ -7,6 +7,7 @@ import ( "runtime" "strings" "testing" + "unicode/utf8" "github.com/stretchr/testify/assert" ) @@ -561,3 +562,116 @@ func TestAssignedWorkerScriptPath(t *testing.T) { }) } } + +func TestSplitPosPrefixSearch(t *testing.T) { + t.Parallel() + tests := []struct { + name string + path string + markers []string + want int + }{ + {"letter prefix folds case", "/INDEX.PHP/info", []string{"php"}, 10}, + {"single letter folds case", "/P/info", []string{"p"}, 2}, + {"single punctuation", "/a./info", []string{"."}, 3}, + {"nonmatching prefix before match", "/a.phx/b.PHP/info", []string{".php"}, 12}, + {"last possible start", "/a.php", []string{".php"}, 6}, + {"marker longer than path", "/a", []string{".php"}, -1}, + {"invalid first occurrence stops marker", "/a.php.txt/b.php", []string{".php"}, -1}, + {"later marker after invalid occurrence", "/a.php.txt/b.phtml", []string{".php", ".phtml"}, 18}, + {"marker order wins over path order", "/a.php/b.phtml", []string{".phtml", ".php"}, 14}, + {"empty marker skipped", "/a.php", []string{"", ".php"}, 6}, + {"nil markers", "/a.php", nil, 0}, + {"invalid UTF8 before extension", "/\xff.PHP/info", []string{".php"}, 6}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := splitPos(tt.path, tt.markers); got != tt.want { + t.Fatalf("splitPos(%q, %q) = %d, want %d", tt.path, tt.markers, got, tt.want) + } + }) + } +} + +// Keep the original scan as an independent fuzzing oracle. +func splitPosReference(path string, splitPath []string) int { + if len(splitPath) == 0 { + return 0 + } + + pathLen := len(path) + + for _, split := range splitPath { + splitLen := len(split) + if splitLen == 0 || splitLen > pathLen { + continue + } + + for i := 0; i <= pathLen-splitLen; i++ { + match := true + for j := range splitLen { + c := path[i+j] + if c >= utf8.RuneSelf { + match = false + + break + } + + if 'A' <= c && c <= 'Z' { + c += 'a' - 'A' + } + + if c != split[j] { + match = false + + break + } + } + + if match { + pos := i + splitLen + // the split must end the path or a whole path segment: without + // this, ".php" embedded in a filename ("/a.php.txt/b.php") + // would split inside that filename and run a script the router + // never matched. Give up on this split string entirely rather + // than looking for a later occurrence of it, so the result + // stays identical to Caddy's file matcher (firstSplit). + if pos != pathLen && path[pos] != '/' { + break + } + + return pos + } + } + } + + return -1 +} + +func FuzzSplitPosEquivalent(f *testing.F) { + for _, seed := range []struct{ path, first, second string }{ + {"/index.php/info", ".php", ".phtml"}, + {"/a.php.txt/b.phtml", ".php", ".phtml"}, + {"/INDEX.PHP/info", "php", ".php"}, + {"/a.php/b.phtml", ".phtml", ".php"}, + {"/shell.php", ".php", ""}, + {"/\xff.PHP", ".php", "\xff"}, + {"", "", ""}, + } { + f.Add(seed.path, seed.first, seed.second) + } + f.Fuzz(func(t *testing.T, path, first, second string) { + markers := []string{strings.ToLower(first), strings.ToLower(second)} + for _, marker := range markers { + for i := range len(marker) { + if marker[i] >= utf8.RuneSelf { + t.Skip() + } + } + } + got, want := splitPos(path, markers), splitPosReference(path, markers) + if got != want { + t.Fatalf("splitPos(%q, %q) = %d, reference = %d", path, markers, got, want) + } + }) +}