Skip to content

deepclone_from_array: guard PHP_INT_MIN ref ids and non-object unserialize#19

Merged
nicolas-grekas merged 1 commit into
mainfrom
from-array-malformed-guards
Jun 8, 2026
Merged

deepclone_from_array: guard PHP_INT_MIN ref ids and non-object unserialize#19
nicolas-grekas merged 1 commit into
mainfrom
from-array-malformed-guards

Conversation

@nicolas-grekas

@nicolas-grekas nicolas-grekas commented Jun 8, 2026

Copy link
Copy Markdown
Member

deepclone_from_array() reconstructs a value graph from an array payload, the decode side of the extension's serialize/deserialize pair. Two malformed-input bugs in that decoder:

Non-object unserialize() result. A class-name entry whose second byte is : is replayed through php_var_unserialize() (the C:/O: wire form the encoder emits for Serializable / __PHP_Incomplete_Class). The result was stored in the objects[] table and later dereferenced as a zend_object* with no type check, so a payload carrying a scalar/array form (i:, s:, a:, ...) made the decoder treat a non-object as an object pointer. The result is now required to be IS_OBJECT; otherwise the documented \ValueError is thrown.

PHP_INT_MIN ref-id negation. Three id-resolution sites (the object-reference and named-closure paths in dc_resolve(), and the top-level prepared loop) compute -id on a signed zend_long to turn a negative ref id into a positive hash key. For id == ZEND_LONG_MIN that negation is signed-overflow UB: benign on two's-complement hardware (it wraps and the lookup misses, yielding a clean \ValueError), but it aborts under -fno-sanitize-recover=undefined builds, which the fuzzer uses. The guard already present on the hard-ref path is now applied to the three sibling sites.

New .phpts cover both, asserting a clean \ValueError on the malformed payloads and unchanged behaviour for ordinary negative ref ids.

Companion polyfill fix: symfony/polyfill#627.

…alize

Three id-resolution sites (the object-reference and named-closure paths in dc_resolve(), and the top-level "prepared" loop) negate a signed ref id to turn it into a positive hash key. For id == ZEND_LONG_MIN that negation is signed-overflow UB; the guard already present on the hard-ref path is now applied to the three sibling sites.

When materialising an object whose class-name blob unserialises a "C:"/"O:" form, the result was stored and later dereferenced as a zend_object* with no type check, so a scalar/array serialize form (i:, s:, a:, ...) was treated as an object pointer. The result is now required to be IS_OBJECT, otherwise a \ValueError is thrown.

Adds .phpt coverage for both, asserting a clean \ValueError on the malformed payloads and unchanged behaviour for ordinary negative ref ids.
@nicolas-grekas nicolas-grekas force-pushed the from-array-malformed-guards branch from 48dd464 to b085deb Compare June 8, 2026 16:53
@nicolas-grekas nicolas-grekas merged commit b085deb into main Jun 8, 2026
19 checks passed
nicolas-grekas added a commit to symfony/polyfill that referenced this pull request Jun 8, 2026
…thout warnings (nicolas-grekas)

This PR was merged into the 1.x branch.

Discussion
----------

[DeepClone] Reject malformed deepclone_from_array() input without warnings

| Q             | A
| ------------- | ---
| Branch?       | 1.x
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Brings the polyfill in line with the extension's handling of malformed `deepclone_from_array()` payloads:

- A serialized class-name blob that does not `unserialize()` to an object (e.g. an `i:`, `s:` or `a:` form) is now rejected with a `\ValueError` instead of being stored and treated as an object.
- A `PHP_INT_MIN` reference id on the object-reference, named-closure and `prepared` paths no longer reaches `-$id`, which overflows to a float and emits a runtime warning before the error. The hard-ref paths and the properties-loop object-ref path were already guarded; the remaining three sites now throw a clean `\ValueError` too.

Companion extension fix: symfony/php-ext-deepclone#19.

Commits
-------

ab240c8 [DeepClone] Reject malformed deepclone_from_array() input without warnings
symfony-splitter pushed a commit to symfony/polyfill-deepclone that referenced this pull request Jun 8, 2026
…nings

A serialized class-name blob that does not unserialize() to an object is now rejected with a \ValueError instead of being stored and treated as an object. A PHP_INT_MIN reference id on the object-reference, named-closure and "prepared" paths no longer reaches -$id, which overflows to a float and emits a runtime warning before the error. Mirrors symfony/php-ext-deepclone#19.
@nicolas-grekas nicolas-grekas deleted the from-array-malformed-guards branch June 8, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant