fix: path confusion via unicode casing in CGI path splitting

Author: dunglas

caddy/mercure.go

@@ -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

caddy/module.go

@@ -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,31 +217,14 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 		}
 	}
 
-	var (
-		err error
-		fr  *http.Request
-	)
-
-	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),
+	fr, err := frankenphp.NewRequestWithContext(
+		r,
+		append(
+			f.requestOptions,
 			frankenphp.WithOriginalRequest(&origReq),
 			frankenphp.WithWorkerName(workerName),
-			*f.mercureHubRequestOption,
-		)
-	}
+		)...,
+	)
 
 	if err != nil {
 		return caddyhttp.Error(http.StatusInternalServerError, err)

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
 }
 

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")
+	}
+}