Block PHP execution outside index.php in the lighttpd and IIS samples - #264
Merged
Merged
Conversation
The Apache configuration Winter ships carries an explicit "Block all PHP files, except index" rule, and the nginx sample reaches the PHP handler only for /index.php. The lighttpd and IIS samples had no equivalent: both serve the public storage paths as-is, so a PHP file written anywhere under them would be handed to the PHP handler. Adds the same restriction to both, placed ahead of the passthrough rules so it applies to the paths those rules serve directly, and notes on the nginx PHP location why it deliberately matches index.php alone. Untested against live lighttpd and IIS servers.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
The Apache configuration Winter ships carries an explicit rule:
and the nginx sample achieves the same thing structurally, since
location ~ ^/index.phpis the only location that reaches FastCGI.The lighttpd and IIS samples had no equivalent. Both deliberately serve the public storage paths and the asset paths as-is — that is the point of those rules — but as a result a PHP file written anywhere underneath them is handed to the PHP handler rather than being routed to the front controller. The other two configurations do not behave that way, so this brings all four in line.
Changes
.phprule as the first entry inurl.rewrite-once. Ordering matters:url.rewrite-onceis first-match-wins, and the passthrough rules below it return their paths unchanged, so the restriction has to be evaluated before them.Block all PHP files, except indexrule ahead of the existing catch-all, withstopProcessing="true", matching\.php$and negatingindex.php./index.phpalone on purpose, so it does not get broadened tolocation ~ \.php$later.Testing
Both rules are written against the documented behaviour of each server (lighttpd's PCRE
url.rewrite-onceand the IIS URL Rewrite module) and reviewed against the existing rules they sit beside, but I have not run them against live lighttpd or IIS servers — I do not have either to hand. A review from someone who deploys on those would be welcome, particularly on the lighttpd negative-lookahead syntax.Defence in depth rather than a fix for a known exploit path: nothing in Winter writes PHP into those directories.