Repository navigation
Conversation
These three recursion paths were missing ZEND_CHECK_STACK_LIMIT, so deeply nested input exhausted the stack and killed the process instead of raising the catchable Error that serialize() already produces. Guard each recursive entry point the same way the already-fixed paths do.
…ards Covers all three entry points with zend.max_allowed_stack_size lowered, asserting each raises the catchable Error instead of exhausting the stack.
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.
Fixes #24605.
ZEND_CHECK_STACK_LIMITguards the recursion inserialize(),var_dump()anddebug_zval_dump(),but not in
var_export(),print_r()orunserialize(). On a structure that fits the stack thosefunctions recurse until it is exhausted and the process dies with SIGSEGV - no error, no message.
Reproduced first
Each function run in its own process, stock
php:8.5-cli,--ulimit stack=524288:524288, 3,000 levels:serialize()Error(already guarded)var_export()print_r()unserialize()Change
The same guard the already-fixed paths use, at each recursive entry point:
ext/standard/var.c-php_var_export_ex()(return FAILURE;)Zend/zend.c-zend_print_zval_r_to_buf(), the recursion behindprint_r(), which lives in Zendrather than ext/standard (
return;)ext/standard/var_unserializer.re-php_var_unserialize_internal()(return 0;)Each site returns the type its function already uses, so no signatures change. 3 files, +18 lines.
After
Built from current master with the patch applied, same structure, same 512 KB stack:
All four now raise the same catchable
Error, and the surrounding behaviour is unchanged forstructures that fit.
Scope I did not test
unserialize()'s ownmax_depthoption (default 4096) usually rejects deep input first, so the newguard only matters when the stack is small relative to that limit - which is the case tested here.
I did not test a very large
max_depthagainst a large stack.