Limit maximum number of filter chains - #22110
Open
Sjord wants to merge 14 commits into
Open
Conversation
Chaining filters is becoming an increasingly popular primitive to exploit PHP applications. Limiting the usage of only a few of them at the time should, if not close entirely, make it significantly less attractive. This should close php#10453
Limit number of filters that can be chained in a php://filter URL. Count number of filters already on the stream, instead of counting iterations on the loop. When filters are separated by slash instead of pipe, php_stream_apply_filter_list is called muliple times, so counting iterations won't work. Instead, count numbers of filters already on the chain. Add more elaborate test that tests: - file read - file include - no warning on stream_filter_append Related to: php#10453 php#16699
added 2 commits
May 22, 2026 11:46
Read the maximum number of filters from context stream. If it is set, it doesn't make sense to raise a deprecation warning, because this stream context option didn't exist earlier. We raise an error consistent with current stream handling, as long as rfc/stream_errors has not landed. If it is not set, we give a deprecation warning when the 17th filter is added. This is unfortunately done in another place; to prevent showing warnings multiple times, the counting is done within the loop and not at the end.
We get a zend_long from the stream context, so it makes sense to consistently use it for all counts of filters.
added 3 commits
June 8, 2026 13:28
Parallel to this work, the stream_errors RFC was accepted and merged. Use the new API php_stream_wrapper_log_warn to report the error of too many filters.
Contributor
Author
|
It's looking pretty good for the RFC, so it would be nice to get a review of this PR. |
Contributor
Author
|
RFC is accepted. @bukka could you review this? |
Member
|
Will check this or next weekend hopefully. |
Contributor
Author
|
@bukka Could you review this please? |
Contributor
Author
|
@LamentXU123 , @arnaud-lb Could one of you perhaps review this pull request? |
LamentXU123
reviewed
Jul 16, 2026
LamentXU123
left a comment
LamentXU123
left a comment
Member
There was a problem hiding this comment.
Sorry for the late response and thank you for your work.
I am not an expert in this topic so just my 2 cents
When both read_chain and write_chain are true, also count the number of write filters and give a deprecation warning when that is exceeded.
Do the counting in php_stream_apply_filter_list and stop adding filters when the limit is reached, instead of adding them all first.
Member
|
@Sjord Thanks. Now please wait for others reviews on this topic because i am not an expert on this.Also keep my reviews unresolved because I want to hear from others opinions. This looks good to me :) |
arnaud-lb
reviewed
Jul 16, 2026
TimWolla
requested changes
Jul 16, 2026
TimWolla
left a comment
TimWolla
left a comment
Member
There was a problem hiding this comment.
Didn't actually look at the code, but these stood out. There might be more cases where you're using spaces instead of tabs.
Instead of counting the number of items in the chain every time we add a filter to the chain, keep an attribute `count` to determine the number of filters already on the chain.
Negative values now disable the limit.
arnaud-lb
approved these changes
Jul 17, 2026
arnaud-lb
left a comment
arnaud-lb
left a comment
Member
There was a problem hiding this comment.
Looks good to me otherwise!
LamentXU123
approved these changes
Jul 18, 2026
LamentXU123
left a comment
LamentXU123
left a comment
Member
There was a problem hiding this comment.
Awaiting Jakub for final review 👍
Contributor
Author
|
@bukka Could you review/merge this? |
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
6 participants
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.
This is an implementation suggestion for the RFC to limit the maximum number of filters in a
php://filterURL.