Repository navigation
fastcgi: reject paths whose escaped form selects another script - #8357
Merged
francislavoie merged 1 commit intoNov 4, 2026
Merged
Conversation
steadytao
requested changes
Nov 4, 2026
Member
|
+ conflicts |
Path matchers whose pattern contains "%" clean the escaped path, where "%2f" and "%2e%2e" are neither separators nor dot segments, while buildEnv cleans the decoded one. With "@public path /public/%*", "/public/tool.php%2f..%2f..%2fadmin%2ftool.php" matched the public rule but ran admin/tool.php. Requests whose escaped and decoded paths select different scripts now get a 400. On Windows those matchers read a backslash as "%2f". Go escapes a backslash even when RawPath is empty, so "%5C..%5C" needs the same check, and backslash separators are folded before comparing so that "/dir\index.php" is still accepted. Backslash dot segments, which the Windows test expected to resolve, now get a 400 like "%2f..%2f". Backport of the FrankenPHP fix reported by Daybreak Red.
dunglas
force-pushed
the
fastcgi-escaped-path-script
branch
from
November 4, 2026 20:29
316983a to
ab50798
Compare
steadytao
approved these changes
Nov 4, 2026
Member
|
Thank you for both PRs! |
Collaborator
Author
|
Thank you for your prompt review, and to FrankenPHP commiters for having reviewed the inital fix :) |
francislavoie
pushed a commit
that referenced
this pull request
Nov 5, 2026
(cherry picked from commit 3d00544)
5 tasks
dunglas
added a commit
to php/frankenphp
that referenced
this pull request
Nov 5, 2026
**Security fix (Windows only).** Follow-up to 393e007. On Windows, Caddy path matchers whose pattern contains `%` read a backslash as `%2f`, which they do not resolve as a separator. Go escapes a backslash even when `RawPath` is empty, so `escapedPathSelectsAnotherScript` returned early for `/public/tool.php%5C..%5C..%5Cadmin%5Ctool.php`: a `/public/%*` rule matched it, but `admin/tool.php` ran. The check now also runs when the path contains a backslash on Windows. Backslashes are folded into separators before comparing, which also fixes a regression: legitimate paths such as `/dir\index.php` were rejected with a 400. Found while porting the fix to Caddy: caddyserver/caddy#8357 ## Assistance Disclosure Claude Code (Claude Opus 5.5) found the issue while porting the fix to Caddy, and wrote the patch and tests. I reviewed and tested the result.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Path matchers whose pattern contains
%clean the escaped path, where%2fand%2e%2eare neither separators nor dot segments, whilebuildEnvcleans the decoded one. With@public path /public/%*,/public/tool.php%2f..%2f..%2fadmin%2ftool.phpmatches the public rule but runsadmin/tool.php.Requests whose escaped and decoded paths select different scripts now get a 400. Two cases of the dot-segment test added in 4ec5a5c3 (
..%2fand%2e%2ebefore the script) moved to the new test, since they now get a 400 instead of running the cleaned script.On Windows, these matchers read a backslash as
%2f. Go escapes a backslash even whenRawPathis empty, so%5C..%5Cgoes through the same check. Backslashes are converted to/before comparing, so/dir\index.phpis still accepted. Backslash dot segments, which the existing Windows test expected to resolve, now get a 400 like%2f..%2f. The same Windows fix for FrankenPHP: php/frankenphp@main...fix/windows-escaped-backslashPort of the FrankenPHP fix: php/frankenphp@393e007
Assistance Disclosure
Claude Code (Claude Opus 5.5) ported this fix from the FrankenPHP patch and wrote the tests. I reviewed and tested the result.