fastcgi: reject paths whose escaped form selects another script (#8357)

modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/url"
 	"path/filepath"
 	"runtime"
 	"strconv"
@@ -322,6 +323,21 @@ func (t Transport) buildEnv(r *http.Request) (envVars, error) {
 		return nil, caddyhttp.Error(http.StatusBadRequest, errors.New("illegal script path"))
 	}
 
+	// Path matchers whose pattern contains "%" clean the escaped path, where "%2f"
+	// and "%2e%2e" are neither separators nor dot segments, so both forms must
+	// select the same script. On Windows, they read a backslash, escaped even
+	// without RawPath, as "%2f".
+	backslash := runtime.GOOS == "windows" && strings.Contains(r.URL.Path, `\`)
+	if r.URL.RawPath != "" || backslash {
+		escaped, err := url.PathUnescape(caddyhttp.NormalizedPath(r.URL.EscapedPath(), true))
+		if backslash {
+			escaped = strings.ReplaceAll(escaped, `\`, "/")
+		}
+		if err != nil || t.scriptPart(escaped) != docURI {
+			return nil, caddyhttp.Error(http.StatusBadRequest, errors.New("escaped and decoded paths select different scripts"))
+		}
+	}
+
 	// Try to grab the path remainder from a file matcher
 	// if we didn't get a split result here.
 	// See https://github.com/caddyserver/caddy/issues/3718
@@ -526,6 +542,14 @@ func (t Transport) splitPos(path string) int {
 	return -1
 }
 
+// scriptPart returns the part of path naming the script.
+func (t Transport) scriptPart(path string) string {
+	if pos := t.splitPos(path); pos > -1 {
+		return path[:pos]
+	}
+	return path
+}
+
 type envVars map[string]string
 
 // loggableEnv is a simple type to allow for speeding up zap log encoding.

modules/caddyhttp/reverseproxy/fastcgi/fastcgi_test.go

@@ -568,18 +568,6 @@ func TestBuildEnvDotSegmentsCannotSelectAnotherScript(t *testing.T) {
 			expectedScriptName: "/other.php",
 			expectedPathInfo:   "",
 		},
-		{
-			name:               "percent-encoded separators",
-			target:             "/admin.php/..%2f..%2fother.php",
-			expectedScriptName: "/other.php",
-			expectedPathInfo:   "",
-		},
-		{
-			name:               "percent-encoded dots",
-			target:             "/admin.php/%2e%2e/other.php",
-			expectedScriptName: "/other.php",
-			expectedPathInfo:   "",
-		},
 		{
 			name:               "dot segments before legitimate path info",
 			target:             "/admin.php/../other.php/extra/path",
@@ -620,3 +608,42 @@ func TestBuildEnvDotSegmentsCannotSelectAnotherScript(t *testing.T) {
 		})
 	}
 }
+
+// Path matchers whose pattern contains "%" clean the escaped path, so
+// "/public/tool.php%2f..%2f..%2fadmin%2ftool.php" matches "path /public/%*"
+// while the decoded path selects admin/tool.php.
+func TestBuildEnvEscapedPathCannotSelectAnotherScript(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		target             string
+		wantErr            bool
+		expectedScriptName string
+	}{
+		{target: "/public/tool.php%2f..%2f..%2fadmin%2ftool.php", wantErr: true},
+		{target: "/admin.php/..%2f..%2fother.php", wantErr: true},
+		{target: "/admin.php/%2e%2e/other.php", wantErr: true},
+		{target: "/dir%2fscript.php/a", expectedScriptName: "/dir/script.php"},
+		{target: "/index.php/a%2f..%2fb", expectedScriptName: "/index.php"},
+		{target: "/index.php/a%2Fb", expectedScriptName: "/index.php"},
+	} {
+		t.Run(tc.target, func(t *testing.T) {
+			r := httptest.NewRequest(http.MethodGet, tc.target, nil)
+			ctx := context.WithValue(r.Context(), caddy.ReplacerCtxKey, caddy.NewReplacer())
+			ctx = context.WithValue(ctx, caddyhttp.VarsCtxKey, map[string]any{})
+			ctx = context.WithValue(ctx, caddyhttp.OriginalRequestCtxKey, *r)
+
+			tr := Transport{Root: t.TempDir(), SplitPath: []string{".php"}}
+			env, err := tr.buildEnv(r.WithContext(ctx))
+			if !tc.wantErr {
+				require.NoError(t, err)
+				assert.Equal(t, tc.expectedScriptName, env["SCRIPT_NAME"])
+				return
+			}
+
+			var herr caddyhttp.HandlerError
+			require.ErrorAs(t, err, &herr)
+			assert.Equal(t, http.StatusBadRequest, herr.StatusCode)
+		})
+	}
+}

modules/caddyhttp/reverseproxy/fastcgi/fastcgi_windows_test.go

@@ -31,7 +31,8 @@ import (
 // On Windows the path matchers additionally fold backslashes into separators and
 // trim per-component trailing dots and spaces (see caddyhttp.NormalizedPath),
 // so the transport must do the same or the dot-segment bypass simply reappears
-// spelled with backslashes.
+// spelled with backslashes. Backslash dot segments are rejected outright because
+// the "%" path matchers read them as "%2f..%2f", which they do not resolve.
 func TestBuildEnvWindowsSeparatorsCannotSelectAnotherScript(t *testing.T) {
 	t.Parallel()
 
@@ -46,21 +47,31 @@ func TestBuildEnvWindowsSeparatorsCannotSelectAnotherScript(t *testing.T) {
 	for _, tc := range []struct {
 		name               string
 		target             string
+		wantErr            bool
 		expectedScriptName string
 		expectedPathInfo   string
 	}{
-		{"backslash dot segments", `/admin.php\..\other.php`, "/other.php", ""},
-		{"backslash, two levels", `/admin.php\..\..\other.php`, "/other.php", ""},
-		{"mixed separators", `/admin.php\../other.php`, "/other.php", ""},
+		{"backslash dot segments", `/admin.php\..\other.php`, true, "", ""},
+		{"backslash, two levels", `/admin.php\..\..\other.php`, true, "", ""},
+		{"mixed separators", `/admin.php\../other.php`, true, "", ""},
+		{"escaped backslash dot segments", "/admin.php%5C..%5Cother.php", true, "", ""},
+		{"backslash separator", `/dir\other.php\a`, false, "/dir/other.php", "/a"},
+		{"escaped backslash separator", "/dir%5Cother.php%5Ca", false, "/dir/other.php", "/a"},
 		// Per-component trailing dot/space folding is Windows-only and is not
 		// something CleanPath does, so this case fails unless the transport
 		// applies the same normalization the matchers do.
-		{"trailing dot in a path-info component", "/other.php/a./b", "/other.php", "/a/b"},
-		{"trailing space in a path-info component", "/other.php/a%20/b", "/other.php", "/a/b"},
+		{"trailing dot in a path-info component", "/other.php/a./b", false, "/other.php", "/a/b"},
+		{"trailing space in a path-info component", "/other.php/a%20/b", false, "/other.php", "/a/b"},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			tr := Transport{Root: `C:\srv`, SplitPath: []string{".php"}}
 			env, err := tr.buildEnv(newRequest(tc.target))
+			if tc.wantErr {
+				var herr caddyhttp.HandlerError
+				require.ErrorAs(t, err, &herr)
+				assert.Equal(t, http.StatusBadRequest, herr.StatusCode)
+				return
+			}
 			require.NoError(t, err)
 
 			assert.Equal(t, tc.expectedScriptName, env["SCRIPT_NAME"],