mirror of
https://github.com/php/frankenphp.git
synced 2026-03-23 16:42:13 +01:00
fix: path confusion via unicode casing in CGI path splitting
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
59
cgi.go
@@ -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
|
||||
}
|
||||
|
||||
|
||||
177
cgi_test.go
177
cgi_test.go
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
69
request_options_test.go
Normal 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)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user