Repository navigation
Fix GH-24492 (partial): route caller-sized string builders through the fallible allocator - #24548
Open
VijhnAustin wants to merge 1 commit into
Open
VijhnAustin wants to merge 1 commit into
VijhnAustin wants to merge 1 commit into
Conversation
… the fallible allocator
Several ext/standard functions grow their output buffer with the non-fallible
string allocators. When the requested size crosses the soft memory_limit the
allocation is not reported back to the caller; it escalates to the hard limit
(memory_hard_limit_percent) and the process is terminated via _exit(137), so
registered shutdown functions never run and the catchable "Allowed memory size
exhausted" fatal that 8.5.11 produced for the same input is lost.
Route these sites through the fallible allocation API already in the tree
(zend_string_fextend / zend_string_falloc / zend_string_safe_falloc with
fallible=true), propagate the NULL return, and surface the standard engine OOM
via RETURN_OOM() at the userland boundary:
- sprintf/printf/vprintf/vsprintf/fprintf/vfprintf (formatted_print.c; the
php_sprintf_append* helpers now return bool)
- pack (pack.c)
- bin2hex and utf8_encode (string.c)
- number_format (math.c; tmpbuf released on the failure path)
- convert_uuencode (uuencode.c)
- quoted_printable_encode (quot_print.c)
Adds 17 .phpt regression tests, each pinning memory_hard_limit_percent=200 and
asserting both the catchable fatal and that a shutdown function still runs.
Other sites named in the issue (base64_encode, urlencode/rawurlencode,
var_export, the bcmath family, and the main/spprintf.c PAD_CHAR path) require
auditing their internal callers for NULL-safety before conversion and are
intentionally left for follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Oct 23, 2026
research: ASan baseline and evidence-backed cross-version OOM comparisons
forgeblueai/php-core-lab#2
Open
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 part of #24492.
Problem
Several
ext/standardfunctions grow their output buffer with the non-fallible string allocators. When the requested size crosses the softmemory_limit, the allocation is not reported back to the caller; it escalates to the hard limit (memory_hard_limit_percent) and the process is terminated via_exit(137), so registered shutdown functions never run and the catchable "Allowed memory size … exhausted" fatal that 8.5.11 produced for the same input is lost. A single userland call with an attacker-influenced size is enough.Fix
Route these sites through the fallible allocation API already in the tree (
zend_string_fextend/zend_string_falloc/zend_string_safe_fallocwithfallible = true), propagate theNULL, and surface the standard engine OOM viaRETURN_OOM()at the userland boundary:sprintf/printf/vprintf/vsprintf/fprintf/vfprintf(formatted_print.c; thephp_sprintf_append*helpers now returnbool)pack(pack.c)bin2hexandutf8_encode(string.c)number_format(math.c;tmpbufreleased on the failure path)convert_uuencode(uuencode.c)quoted_printable_encode(quot_print.c)Scope
This is a partial fix. Other sites named in the issue —
base64_encode,urlencode/rawurlencode,var_export, the bcmath family, and themain/spprintf.cPAD_CHAR path — route their result throughPHPAPIhelpers whose internal callers currently dereference the result without a NULL check (they never see a failed huge allocation today because the process is killed first). Converting those safely needs a caller audit, so they are intentionally left for follow-up. Theext/xml/Expat variant is already handled in #24516.Reproduction
With
memory_limit=16Mandmemory_hard_limit_percent=200:Before (stock):
Hard memory limit of 33554432 bytes exhausted …, exit 137, no shutdown.After (patched): catchable
Allowed memory size of 16777216 bytes exhausted …, shutdown runs, exit 255.Tests
Adds 17
.phptunderext/standard/tests/{strings,math}/gh24492_*, each pinningmemory_hard_limit_percent=200and asserting both the catchable fatal and that a shutdown function still runs (so a failure while preparing the input cannot masquerade as a reproduced bug). Verified on a Windows x64 build (17/17 passing); the patch applies cleanly to currentPHP-8.5. Also reproduces on 8.4.27 per the issue — happy to retarget toPHP-8.4if that is preferred.🤖 Generated with Claude Code