fix: path confusion via unicode casing in CGI path splitting

This commit is contained in:
Kévin Dunglas
2026-01-26 17:21:34 +01:00
parent 711d03256e
commit 04fdc0c1e8
6 changed files with 350 additions and 47 deletions

View File

@@ -22,8 +22,7 @@ func (f *FrankenPHPModule) assignMercureHub(ctx caddy.Context) {
return
}
opt := frankenphp.WithMercureHub(f.mercureHub)
f.mercureHubRequestOption = &opt
f.requestOptions = append(f.requestOptions, frankenphp.WithMercureHub(f.mercureHub))
for i, wc := range f.Workers {
wc.mercureHub = f.mercureHub

View File

@@ -51,7 +51,7 @@ type FrankenPHPModule struct {
preparedEnv frankenphp.PreparedEnv
preparedEnvNeedsReplacement bool
logger *slog.Logger
mercureHubRequestOption *frankenphp.RequestOption
requestOptions []frankenphp.RequestOption
}
// CaddyModule returns the Caddy module information.
@@ -118,6 +118,7 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
if len(f.SplitPath) == 0 {
f.SplitPath = []string{".php"}
}
f.requestOptions = append(f.requestOptions, frankenphp.WithRequestSplitPath(f.SplitPath))
if f.ResolveRootSymlink == nil {
rrs := true
@@ -148,6 +149,8 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
f.Workers[i].FileName = resolvedPath
}
}
f.requestOptions = append(f.requestOptions, frankenphp.WithRequestResolvedDocumentRoot(f.resolvedDocumentRoot))
}
if f.preparedEnv == nil {
@@ -162,6 +165,10 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
}
}
if !f.preparedEnvNeedsReplacement {
f.requestOptions = append(f.requestOptions, frankenphp.WithRequestPreparedEnv(f.preparedEnv))
}
if err := f.configureHotReload(fapp); err != nil {
return err
}
@@ -180,31 +187,26 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
origReq := ctx.Value(caddyhttp.OriginalRequestCtxKey).(http.Request)
repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
var (
documentRootOption frankenphp.RequestOption
documentRoot string
)
if f.resolvedDocumentRoot == "" {
documentRoot := f.resolvedDocumentRoot
if documentRoot == "" {
documentRoot = repl.ReplaceKnown(f.Root, "")
if documentRoot == "" && frankenphp.EmbeddedAppPath != "" {
documentRoot = frankenphp.EmbeddedAppPath
}
// If we do not have a resolved document root, then we cannot resolve the symlink of our cwd because it may
// resolve to a different directory than the one we are currently in.
// This is especially important if there are workers running.
documentRootOption = frankenphp.WithRequestDocumentRoot(documentRoot, false)
} else {
documentRoot = f.resolvedDocumentRoot
documentRootOption = frankenphp.WithRequestResolvedDocumentRoot(documentRoot)
f.requestOptions = append(f.requestOptions, frankenphp.WithRequestDocumentRoot(documentRoot, false))
}
env := f.preparedEnv
if f.preparedEnvNeedsReplacement {
env = make(frankenphp.PreparedEnv, len(f.Env))
env := make(frankenphp.PreparedEnv, len(f.Env))
for k, v := range f.preparedEnv {
env[k] = repl.ReplaceKnown(v, "")
}
f.requestOptions = append(f.requestOptions, frankenphp.WithRequestPreparedEnv(env))
}
workerName := ""
@@ -215,32 +217,15 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
}
}
var (
err error
fr *http.Request
fr, err := frankenphp.NewRequestWithContext(
r,
append(
f.requestOptions,
frankenphp.WithOriginalRequest(&origReq),
frankenphp.WithWorkerName(workerName),
)...,
)
if f.mercureHubRequestOption == nil {
fr, err = frankenphp.NewRequestWithContext(
r,
documentRootOption,
frankenphp.WithRequestSplitPath(f.SplitPath),
frankenphp.WithRequestPreparedEnv(env),
frankenphp.WithOriginalRequest(&origReq),
frankenphp.WithWorkerName(workerName),
)
} else {
fr, err = frankenphp.NewRequestWithContext(
r,
documentRootOption,
frankenphp.WithRequestSplitPath(f.SplitPath),
frankenphp.WithRequestPreparedEnv(env),
frankenphp.WithOriginalRequest(&origReq),
frankenphp.WithWorkerName(workerName),
*f.mercureHubRequestOption,
)
}
if err != nil {
return caddyhttp.Error(http.StatusInternalServerError, err)
}

59
cgi.go
View File

@@ -18,9 +18,12 @@ import (
"net/http"
"path/filepath"
"strings"
"unicode/utf8"
"unsafe"
"github.com/dunglas/frankenphp/internal/phpheaders"
"golang.org/x/text/language"
"golang.org/x/text/search"
)
// Protocol versions, in Apache mod_ssl format: https://httpd.apache.org/docs/current/mod/mod_ssl.html
@@ -252,24 +255,64 @@ func splitCgiPath(fc *frankenPHPContext) {
fc.worker = getWorkerByPath(fc.scriptFilename)
}
// splitPos returns the index where path should
// be split based on SplitPath.
var splitSearchNonASCII = search.New(language.Und, search.IgnoreCase)
// splitPos returns the index where path should be split based on splitPath.
// example: if splitPath is [".php"]
// "/path/to/script.php/some/path": ("/path/to/script.php", "/some/path")
//
// Adapted from https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
// Copyright 2015 Matthew Holt and The Caddy Authors
func splitPos(path string, splitPath []string) int {
if len(splitPath) == 0 {
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 WithRequestSplitPath
for _, split := range 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
}

View File

@@ -1,6 +1,7 @@
package frankenphp
import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
@@ -31,3 +32,179 @@ func TestEnsureLeadingSlash(t *testing.T) {
})
}
}
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 := splitPos(tt.path, tt.splitPath)
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 := splitPos(path, split)
// 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")
}
}

View File

@@ -1,11 +1,14 @@
package frankenphp
import (
"errors"
"log/slog"
"net/http"
"path/filepath"
"strings"
"sync"
"sync/atomic"
"unicode/utf8"
"github.com/dunglas/frankenphp/internal/fastabs"
)
@@ -14,6 +17,8 @@ import (
type RequestOption func(h *frankenPHPContext) error
var (
ErrInvalidSplitPath = errors.New("split path contains non-ASCII characters")
documentRootCache sync.Map
documentRootCacheLen atomic.Uint32
)
@@ -71,11 +76,36 @@ func WithRequestResolvedDocumentRoot(documentRoot string) RequestOption {
// 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.
func WithRequestSplitPath(splitPath []string) RequestOption {
return func(o *frankenPHPContext) error {
var b strings.Builder
for i, split := range 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)
}
}
splitPath[i] = b.String()
b.Reset()
}
o.splitPath = splitPath
return nil

69
request_options_test.go Normal file
View File

@@ -0,0 +1,69 @@
package frankenphp
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestWithRequestSplitPath(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) {
ctx := &frankenPHPContext{}
opt := WithRequestSplitPath(tt.splitPath)
err := opt(ctx)
if tt.wantErr != nil {
require.ErrorIs(t, err, tt.wantErr)
return
}
require.NoError(t, err)
assert.Equal(t, tt.wantSplitPath, ctx.splitPath)
})
}
}