From 7c28c0c07ac70a8960a166c7126150a408ba7464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 10 Feb 2026 19:52:36 +0100 Subject: [PATCH] Merge commit from fork * fix: FastCGI split SCRIPT_NAME/PATH_INFO confusion * fix comment --- .../caddyhttp/reverseproxy/fastcgi/fastcgi.go | 90 ++++++- .../reverseproxy/fastcgi/fastcgi_test.go | 246 ++++++++++++++++++ 2 files changed, 332 insertions(+), 4 deletions(-) create mode 100644 modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index 5c68c3ad5..c60da897b 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -16,6 +16,7 @@ package fastcgi import ( "crypto/tls" + "errors" "fmt" "net" "net/http" @@ -23,9 +24,12 @@ import ( "strconv" "strings" "time" + "unicode/utf8" "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" @@ -33,7 +37,11 @@ import ( "github.com/caddyserver/caddy/v2/modules/caddytls" ) -var noopLogger = zap.NewNop() +var ( + ErrInvalidSplitPath = errors.New("split path contains non-ASCII characters") + + noopLogger = zap.NewNop() +) func init() { caddy.RegisterModule(Transport{}) @@ -50,6 +58,9 @@ type Transport struct { // actual resource (CGI script) name, and the second piece will be set to // PATH_INFO for the CGI script to use. // + // Split paths can only contain ASCII characters. + // Comparison is case-insensitive. + // // Future enhancements should be careful to avoid CVE-2019-11043, // which can be mitigated with use of a try_files-like behavior // that 404s if the fastcgi path info is not found. @@ -109,6 +120,28 @@ func (t *Transport) Provision(ctx caddy.Context) error { t.DialTimeout = caddy.Duration(3 * time.Second) } + var b strings.Builder + + for i, split := range t.SplitPath { + b.Grow(len(split)) + + for j := 0; j < len(split); j++ { + c := split[j] + if c >= utf8.RuneSelf { + return ErrInvalidSplitPath + } + + if 'A' <= c && c <= 'Z' { + b.WriteByte(c + 'a' - 'A') + } else { + b.WriteByte(c) + } + } + + t.SplitPath[i] = b.String() + b.Reset() + } + return nil } @@ -385,8 +418,15 @@ 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") +// +// Adapted from FrankenPHP's code (copyright 2026 Kévin Dunglas, MIT license) func (t Transport) splitPos(path string) int { // TODO: from v1... // if httpserver.CaseSensitivePath { @@ -396,12 +436,54 @@ func (t Transport) splitPos(path string) int { return 0 } - lowerPath := strings.ToLower(path) + 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 { - if idx := strings.Index(lowerPath, strings.ToLower(split)); idx > -1 { - return idx + len(split) + splitLen := len(split) + + for i := 0; i < pathLen; i++ { + if path[i] >= utf8.RuneSelf { + if _, end := splitSearchNonASCII.IndexString(path, split); end > -1 { + return end + } + + break + } + + if i+splitLen > pathLen { + continue + } + + match := true + for j := 0; j < splitLen; j++ { + c := path[i+j] + + if c >= utf8.RuneSelf { + if _, end := splitSearchNonASCII.IndexString(path, split); end > -1 { + return end + } + + break + } + + if 'A' <= c && c <= 'Z' { + c += 'a' - 'A' + } + + if c != split[j] { + match = false + + break + } + } + + if match { + return i + splitLen + } } } + return -1 } diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go new file mode 100644 index 000000000..7097ff790 --- /dev/null +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go @@ -0,0 +1,246 @@ +package fastcgi + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/caddyserver/caddy/v2" +) + +func TestProvisionSplitPath(t *testing.T) { + tests := []struct { + name string + splitPath []string + wantErr error + wantSplitPath []string + }{ + { + name: "valid lowercase split path", + splitPath: []string{".php"}, + wantErr: nil, + wantSplitPath: []string{".php"}, + }, + { + name: "valid uppercase split path normalized", + splitPath: []string{".PHP"}, + wantErr: nil, + wantSplitPath: []string{".php"}, + }, + { + name: "valid mixed case split path normalized", + splitPath: []string{".PhP", ".PHTML"}, + wantErr: nil, + wantSplitPath: []string{".php", ".phtml"}, + }, + { + name: "empty split path", + splitPath: []string{}, + wantErr: nil, + wantSplitPath: []string{}, + }, + { + name: "non-ASCII character in split path rejected", + splitPath: []string{".php", ".Ⱥphp"}, + wantErr: ErrInvalidSplitPath, + }, + { + name: "unicode character in split path rejected", + splitPath: []string{".phpⱥ"}, + wantErr: ErrInvalidSplitPath, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tr := Transport{SplitPath: tt.splitPath} + err := tr.Provision(caddy.Context{}) + + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + + return + } + + require.NoError(t, err) + assert.Equal(t, tt.wantSplitPath, tr.SplitPath) + }) + } +} + +func TestSplitPos(t *testing.T) { + tests := []struct { + name string + path string + splitPath []string + wantPos int + }{ + { + name: "simple php extension", + path: "/path/to/script.php", + splitPath: []string{".php"}, + wantPos: 19, + }, + { + name: "php extension with path info", + path: "/path/to/script.php/some/path", + splitPath: []string{".php"}, + wantPos: 19, + }, + { + name: "case insensitive match", + path: "/path/to/script.PHP", + splitPath: []string{".php"}, + wantPos: 19, + }, + { + name: "mixed case match", + path: "/path/to/script.PhP/info", + splitPath: []string{".php"}, + wantPos: 19, + }, + { + name: "no match", + path: "/path/to/script.txt", + splitPath: []string{".php"}, + wantPos: -1, + }, + { + name: "empty split path", + path: "/path/to/script.php", + splitPath: []string{}, + wantPos: 0, + }, + { + name: "multiple split paths first match", + path: "/path/to/script.php", + splitPath: []string{".php", ".phtml"}, + wantPos: 19, + }, + { + name: "multiple split paths second match", + path: "/path/to/script.phtml", + splitPath: []string{".php", ".phtml"}, + wantPos: 21, + }, + // Unicode case-folding tests (security fix for GHSA-g966-83w7-6w38) + // U+023A (Ⱥ) lowercases to U+2C65 (ⱥ), which has different UTF-8 byte length + // Ⱥ: 2 bytes (C8 BA), ⱥ: 3 bytes (E2 B1 A5) + { + name: "unicode path with case-folding length expansion", + path: "/ȺȺȺȺshell.php", + splitPath: []string{".php"}, + wantPos: 18, // correct position in original string + }, + { + name: "unicode path with extension after expansion chars", + path: "/ȺȺȺȺshell.php/path/info", + splitPath: []string{".php"}, + wantPos: 18, + }, + { + name: "unicode in filename with multiple php occurrences", + path: "/ȺȺȺȺshell.php.txt.php", + splitPath: []string{".php"}, + wantPos: 18, // should match first .php, not be confused by byte offset shift + }, + { + name: "unicode case insensitive extension", + path: "/ȺȺȺȺshell.PHP", + splitPath: []string{".php"}, + wantPos: 18, + }, + { + name: "unicode in middle of path", + path: "/path/Ⱥtest/script.php", + splitPath: []string{".php"}, + wantPos: 23, // Ⱥ is 2 bytes, so path is 23 bytes total, .php ends at byte 23 + }, + { + name: "unicode only in directory not filename", + path: "/Ⱥ/script.php", + splitPath: []string{".php"}, + wantPos: 14, + }, + // Additional Unicode characters that expand when lowercased + // U+0130 (İ - Turkish capital I with dot) lowercases to U+0069 + U+0307 + { + name: "turkish capital I with dot", + path: "/İtest.php", + splitPath: []string{".php"}, + wantPos: 11, + }, + // Ensure standard ASCII still works correctly + { + name: "ascii only path with case variation", + path: "/PATH/TO/SCRIPT.PHP/INFO", + splitPath: []string{".php"}, + wantPos: 19, + }, + { + name: "path at root", + path: "/index.php", + splitPath: []string{".php"}, + wantPos: 10, + }, + { + name: "extension in middle of filename", + path: "/test.php.bak", + splitPath: []string{".php"}, + wantPos: 9, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotPos := Transport{SplitPath: tt.splitPath}.splitPos(tt.path) + assert.Equal(t, tt.wantPos, gotPos, "splitPos(%q, %v)", tt.path, tt.splitPath) + + // Verify that the split produces valid substrings + if gotPos > 0 && gotPos <= len(tt.path) { + scriptName := tt.path[:gotPos] + pathInfo := tt.path[gotPos:] + + // The script name should end with one of the split extensions (case-insensitive) + hasValidEnding := false + for _, split := range tt.splitPath { + if strings.HasSuffix(strings.ToLower(scriptName), split) { + hasValidEnding = true + break + } + } + assert.True(t, hasValidEnding, "script name %q should end with one of %v", scriptName, tt.splitPath) + + // Original path should be reconstructable + assert.Equal(t, tt.path, scriptName+pathInfo, "path should be reconstructable from split parts") + } + }) + } +} + +// TestSplitPosUnicodeSecurityRegression specifically tests the vulnerability +// described in GHSA-g966-83w7-6w38 where Unicode case-folding caused +// incorrect SCRIPT_NAME/PATH_INFO splitting +func TestSplitPosUnicodeSecurityRegression(t *testing.T) { + // U+023A: Ⱥ (UTF-8: C8 BA). Lowercase is ⱥ (UTF-8: E2 B1 A5), longer in bytes. + path := "/ȺȺȺȺshell.php.txt.php" + split := []string{".php"} + + pos := Transport{SplitPath: split}.splitPos(path) + + // The vulnerable code would return 22 (computed on lowercased string) + // The correct code should return 18 (position in original string) + expectedPos := strings.Index(path, ".php") + len(".php") + assert.Equal(t, expectedPos, pos, "split position should match first .php in original string") + assert.Equal(t, 18, pos, "split position should be 18, not 22") + + if pos > 0 && pos <= len(path) { + scriptName := path[:pos] + pathInfo := path[pos:] + + assert.Equal(t, "/ȺȺȺȺshell.php", scriptName, "script name should be the path up to first .php") + assert.Equal(t, ".txt.php", pathInfo, "path info should be the remainder after first .php") + } +}