mirror of
https://github.com/caddyserver/caddy.git
synced 2026-05-13 09:06:41 +00:00
Merge commit from fork
Both fallbacks in splitPos relied on golang.org/x/text/search with search.IgnoreCase, which performs Unicode equivalence matching far beyond ASCII case folding. Combined with the validated-ASCII guarantee on every SplitPath entry, that fallback turned non-PHP filenames into PHP scripts: - when the inner loop hit a non-ASCII byte and the IndexString fallback returned -1, the loop broke without resetting match=false, so a stale match=true caused a non-existent .php to be reported (PoC: "/name.<U+00A1>.txt"). - search.IgnoreCase folded fullwidth, mathematical and circled letters onto ASCII, so "/shell.<math sans-serif php>", "/shell.<fullwidth p>hp", "/shell.<circled php>" were all detected as ".php" files. Replace the fallback with strict byte-level ASCII case-insensitive matching: any byte >= utf8.RuneSelf in the path can never be part of a match, since SplitPath entries are validated ASCII-only and lower-cased in Provision(). This keeps the hot path branch-light and removes the x/text/search dependency from the main module. Reported against FrankenPHP as GHSA-3g8v-8r37-cgjm and GHSA-v4h7-cj44-8fc8. The vulnerable function in this module was adapted from the same FrankenPHP code.
This commit is contained in:
Kévin Dunglas
GitHub
parent
0780d4489c
commit
fb324331f4
3 changed files with 101 additions and 24 deletions
2
go.mod
2
go.mod
|
|
@ -170,7 +170,7 @@ require (
|
|||
go.uber.org/multierr v1.11.0 // indirect
|
||||
golang.org/x/mod v0.35.0 // indirect
|
||||
golang.org/x/sys v0.43.0
|
||||
golang.org/x/text v0.36.0
|
||||
golang.org/x/text v0.36.0 // indirect
|
||||
golang.org/x/tools v0.44.0 // indirect
|
||||
google.golang.org/grpc v1.80.0 // indirect
|
||||
google.golang.org/protobuf v1.36.11 // indirect
|
||||
|
|
|
|||
|
|
@ -28,8 +28,6 @@ import (
|
|||
|
||||
"go.uber.org/zap"
|
||||
"go.uber.org/zap/zapcore"
|
||||
"golang.org/x/text/language"
|
||||
"golang.org/x/text/search"
|
||||
|
||||
"github.com/caddyserver/caddy/v2"
|
||||
"github.com/caddyserver/caddy/v2/modules/caddyhttp"
|
||||
|
|
@ -418,14 +416,19 @@ func (t Transport) buildEnv(r *http.Request) (envVars, error) {
|
|||
return env, nil
|
||||
}
|
||||
|
||||
var splitSearchNonASCII = search.New(language.Und, search.IgnoreCase)
|
||||
|
||||
// splitPos returns the index where path should
|
||||
// be split based on t.SplitPath.
|
||||
//
|
||||
// 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 Provision(), 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
|
||||
// FrankenPHP advisories GHSA-3g8v-8r37-cgjm and GHSA-v4h7-cj44-8fc8.
|
||||
//
|
||||
// Adapted from FrankenPHP's code (copyright 2026 Kévin Dunglas, MIT license)
|
||||
func (t Transport) splitPos(path string) int {
|
||||
// TODO: from v1...
|
||||
|
|
@ -438,31 +441,18 @@ func (t Transport) splitPos(path string) int {
|
|||
|
||||
pathLen := len(path)
|
||||
|
||||
// We are sure that split strings are all ASCII-only and lower-case because of validation and normalization in Provision().
|
||||
for _, split := range t.SplitPath {
|
||||
splitLen := len(split)
|
||||
if splitLen == 0 || splitLen > pathLen {
|
||||
continue
|
||||
}
|
||||
|
||||
for i := range pathLen {
|
||||
if path[i] >= utf8.RuneSelf {
|
||||
if _, end := splitSearchNonASCII.IndexString(path, split); end > -1 {
|
||||
return end
|
||||
}
|
||||
|
||||
break
|
||||
}
|
||||
|
||||
if i+splitLen > pathLen {
|
||||
continue
|
||||
}
|
||||
|
||||
for i := 0; i <= pathLen-splitLen; i++ {
|
||||
match := true
|
||||
for j := range splitLen {
|
||||
for j := 0; j < splitLen; j++ {
|
||||
c := path[i+j]
|
||||
|
||||
if c >= utf8.RuneSelf {
|
||||
if _, end := splitSearchNonASCII.IndexString(path, split); end > -1 {
|
||||
return end
|
||||
}
|
||||
match = false
|
||||
|
||||
break
|
||||
}
|
||||
|
|
|
|||
|
|
@ -191,6 +191,65 @@ func TestSplitPos(t *testing.T) {
|
|||
splitPath: []string{".php"},
|
||||
wantPos: 9,
|
||||
},
|
||||
// Regression tests adapted from FrankenPHP advisories
|
||||
// GHSA-3g8v-8r37-cgjm and GHSA-v4h7-cj44-8fc8: search.IgnoreCase
|
||||
// matched Unicode equivalents of ASCII letters as ".php", and an
|
||||
// inner non-ASCII byte path could leave the match flag stale.
|
||||
{
|
||||
name: "non-ascii byte after dot must not match",
|
||||
path: "/PoC-match-unset.¡.txt",
|
||||
splitPath: []string{".php"},
|
||||
wantPos: -1,
|
||||
},
|
||||
{
|
||||
name: "non-ascii byte mid-extension must not match",
|
||||
path: "/script.p\xc2\xa1p",
|
||||
splitPath: []string{".php"},
|
||||
wantPos: -1,
|
||||
},
|
||||
{
|
||||
name: "small full stop ﹒ in extension must not match",
|
||||
path: "/shell﹒php",
|
||||
splitPath: []string{".php"},
|
||||
wantPos: -1,
|
||||
},
|
||||
{
|
||||
name: "fullwidth full stop ． in extension must not match",
|
||||
path: "/shell．php",
|
||||
splitPath: []string{".php"},
|
||||
wantPos: -1,
|
||||
},
|
||||
{
|
||||
name: "fullwidth p in extension must not match",
|
||||
path: "/shell.ｐhp",
|
||||
splitPath: []string{".php"},
|
||||
wantPos: -1,
|
||||
},
|
||||
{
|
||||
name: "circled php must not match",
|
||||
path: "/shell.ⓟⓗⓟ",
|
||||
splitPath: []string{".php"},
|
||||
wantPos: -1,
|
||||
},
|
||||
{
|
||||
name: "mathematical sans-serif bold php must not match",
|
||||
path: "/shell.\U0001D5FD\U0001D5F5\U0001D5FD",
|
||||
splitPath: []string{".php"},
|
||||
wantPos: -1,
|
||||
},
|
||||
{
|
||||
name: "mathematical script php must not match",
|
||||
path: "/shell.\U0001D4C5\U0001D4BD\U0001D4C5",
|
||||
splitPath: []string{".php"},
|
||||
wantPos: -1,
|
||||
},
|
||||
{
|
||||
name: "circled php with later real php still picks the real one",
|
||||
path: "/shell.ⓟⓗⓟ.anything-after-payload.php",
|
||||
splitPath: []string{".php"},
|
||||
// "/shell." (7) + "ⓟⓗⓟ" (3*3 bytes) + ".anything-after-payload.php" (27) = 43
|
||||
wantPos: 43,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
@ -244,3 +303,31 @@ func TestSplitPosUnicodeSecurityRegression(t *testing.T) {
|
|||
assert.Equal(t, ".txt.php", pathInfo, "path info should be the remainder after first .php")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSplitPosSecurityRegressionUnicodeBypass guards against the FrankenPHP
|
||||
// advisories GHSA-3g8v-8r37-cgjm (uninitialized match flag on inner non-ASCII
|
||||
// byte) and GHSA-v4h7-cj44-8fc8 (Unicode equivalence via search.IgnoreCase
|
||||
// folding fullwidth/mathematical/circled letters onto ASCII). Every payload
|
||||
// below produced a false positive in the vulnerable implementation; none
|
||||
// must match here.
|
||||
func TestSplitPosSecurityRegressionUnicodeBypass(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tr := Transport{SplitPath: []string{".php"}}
|
||||
payloads := []string{
|
||||
"/PoC-match-unset.¡.txt", // GHSA-3g8v: stale match=true on IndexString fallback
|
||||
"/shell﹒php", // U+FE52 small full stop
|
||||
"/shell．php", // U+FF0E fullwidth full stop
|
||||
"/shell.ｐhp", // U+FF50 fullwidth p
|
||||
"/shell.pｈp", // U+FF48 fullwidth h
|
||||
"/shell.phｐ", // U+FF50 fullwidth p (trailing)
|
||||
"/shell.\U0001D5C1\U0001D5B5\U0001D5C1", // mathematical sans-serif p/h
|
||||
"/shell.\U0001D5FD\U0001D5F5\U0001D5FD", // mathematical sans-serif bold p/h
|
||||
"/shell.\U0001D4C5\U0001D4BD\U0001D4C5", // mathematical script p/h
|
||||
"/shell.ⓟⓗⓟ", // circled latin small
|
||||
}
|
||||
|
||||
for _, p := range payloads {
|
||||
assert.Equalf(t, -1, tr.splitPos(p), "payload %q must not be detected as .php", p)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue