Previously, if an object had RC1 it would never be recorded in
php_serialize_data.ht because it was assumed that it could not be encountered
again. This assumption is incorrect though as the object itself may be saved
inside an array with RCn. This results in a new instance of the object, instead
of a second reference to the same object.
This is solved by tracking these objects in php_serialize_data.ht. To retain
performance, track if the current object resides in a potentially nested RCn
array. If not, and if the object is RC1 itself it may be omitted from
php_serialize_data.ht.
Additionally, we may treat the array root itself as RC1 because it may not
appear in the object graph again without recursion. Recursive arrays are still
somewhat broken even with this change, as the tracking of the array only happens
when the reference is encountered, thus resulting in a -> a' -> a' for a self
recursive array a -> a. Recursive arrays have limited support in serialize
anyway, so we ignore this case for now.
Co-authored-by: Dmitry Stogov <dmitry@zend.com>
Co-authored-by: Martin Hoch <martin@littlerobot.de>
Closes GH-11349
Closes GH-11305
* unserialize: Strictly check for `:{` at object start
* unserialize: Update CVE tests
It's unlikely that the object syntax error contributed to the actual CVE. The
CVE is rather caused by the incorrect object serialization data of the `C`
format. Add a second string without such a syntax error to ensure that path is
still executed as well to ensure the CVE is absent.
* Fix test expectation in gmp/tests/bug74670.phpt
No changes to the input required, because the test actually is intended to
verify the behavior for a missing `}`, it's just that the report position changed.
* NEWS
* UPGRADING
* Unserialize: Migrate "Unexpected end of serialized data" to E_WARNING
* Unserialize: Migrate "Error at offset %d of %d bytes" to E_WARNING
* Unserialize: Migrate "%s is returned from __sleep() multiple times" to E_WARNING
* Add NEWS for “Promote unserialize() notices to warning”
* Emit deprecation warnings when adding dynamic properties to classes during unserialization - this will become an Error in php 9.0.
(Adding dynamic properties in other contexts was already a deprecation warning - the use case of unserialization was overlooked)
* Throw an error when attempting to add a dynamic property to a `readonly` class when unserializing
* Add new serialization methods `__serialize`/`__unserialize` for SplFixedArray to avoid creating deprecated dynamic
properties that would then be added to the backing fixed-size array
* Don't add named dynamic/declared properties (e.g. $obj->foo) of SplFixedArray to the backing array when unserializing
* Update tests to declare properties or to expect the deprecation warning
* Add news entry
Co-authored-by: Tyson Andre <tysonandre775@hotmail.com>
While it may not be desired, `DateInterval::$f` supports negative
values, at least with regard to calculations. We still need to guard
from assigning double values which are out of range for signed 64bit
integers (which would be undefined behavior). zend_dval_to_lval() does
this by returning `0` instead of triggering UB. This way we can avoid
setting the invalid marker, which doesn't work as expected anyway.
We must not do that only for unserialization, but also when the property
is set in the first place.
We need to adapt some of the existing tests wrt. this behavior. In
particular, we check for an arbitrary value in bug79015.phpt, to cater
to differences between 32bit and 64bit architectures.
Closes GH-7575.
This API had rather peculiar behavior in case the provided function
is not callable. For some types of failures, it would silently
return FAILURE (e.g. a function does not exist), while for others
(e.g. a class does not exist) it would generate a warning. Depending
on what the calling code does, this can either result in silent
failure or duplicate errors.
This commit switches the contract such that zend_call_function()
always (*) succeeds, though that success might be in the form of
throwing an exception. Calling a non-callable will now consistently
throw an exception.
There are some rare callers that do want to ignore missing methods,
for legacy APIs that are specific with optional methods. For these
use cases a new zend_call_method_if_exists() API is provided.
Calling code generally does not need to explicitly check for and
report zend_call_function() failures -- it can rely on
zend_call_function() having already done so. However, existing
code that does check for failure should continue to work fine.
(*) The only exception to this is if EG(active) being false during
late engine shutdown. This is not relevant to most code, but code
running in destructors and similar may need to be aware of the
possibility.
Add a new interned string handler that fetches an interned string
if it exists, but does not create one if it does not (and instead
returns a non-interned string).
This fixes bug #81142, by preventing the creating of new interned
strings for unserialized array keys.
Closes GH-7360.
The deprecation message was originally introduced in 3e6b447 (#6494).
I first encountered this notice when testing the MongoDB extension
with PHP 8.1, which produced many duplicate messages that provided
no detail about the particular class that needed to be fixed.
Closes GH-7346.
This prevents serialization and unserialization of a class and its
children in a way that does not depend on the zend_class_serialize_deny
and zend_class_unserialize_deny handlers that will be going away
in PHP 9 together with the Serializable interface.
In stubs, `@not-serializable` can be used to set this flag.
This patch only uses the new flag for a handful of Zend classes,
converting the remainder is left for later.
Closes GH-7249.
Fixes bug #81111.
This format matches against null bytes, and prevents the test
expectation from being interpreted as binary data.
bless_tests.php will automatically replace \0 with %0 as well.
The object hash is not particularly useful (anymore) and just
clutters the output. It encodes the same information as the
object ID, which is already part of the output.
If Serializable is implemented, require that __serialize() and
__unserialize() are implemented as well, else issue a deprecation
warning.
Also deprecate use of PDO::FETCH_SERIALIZE.
RFC: https://wiki.php.net/rfc/phase_out_serializable
Closes GH-6494.
The UNDEF marker here is important to prevent the creation of
a reference to the property currently being overwritten, which
would then leak.
This fixes oss-fuzz 6029559193534464, which was incorrectly
merged into oss-fuzz #30584 (which is reported at
https://github.com/google/oss-fuzz/issues/5211).
This restores the previous behavior for this case. We'll continue
to use the mangled name, even if it does not correspond to a
declared property.
This also fixes an assertion failure for the case of property
overwrite, as the add_new was not guaranteed to be "new" previously.
Fixes oss-fuzz #31045.
This restricts allowed usage of $GLOBALS, with the effect that
plain PHP arrays can no longer contain INDIRECT elements.
RFC: https://wiki.php.net/rfc/restrict_globals_usage
Closes GH-6487.
Introduced by the recent switch to a zend_object. Unserialize the
object into a tmp_var to avoid leaving behind a stack reference.
Fixes oss-fuzz #29271.
This was using strcmp instead of zend_string_equals_literal.
As a result, the property count didn't match the number of properties
being serialized if properties started with
"__PHP_Incomplete_Class\0" (unlikely)
(before, `'O:8:"Missing_":1:{}'` would be serialized, which failed to
unserialize)
Everywhere else expects the MAGIC_MEMBER to match exactly,
and this should use zend_string_equals_literal as an example for other code.
This has used strcmp since 2004 in deb84befae
Closes GH-6555
We also need to discard old entries in the ref_props HT when values
are overwritten.
We should really forbid these kinds of overwrites. I believe they
can only occur in manually crafted serialization strings, and
cause so many problems...
Fixes oss-fuzz #28257.
Missed a check for info in this code. Add it, and add an assertion
in type source removal to make it easier to catch this issue.
Fixes oss-fuzz #28208 and #28257.
References to null-serializations are stored as null, and as such
are part of the reference count.
Reminds me that we really need to deprecate the mess that is
Serializable.
Handle one case the previous patch did not account for: If
unserialization of data fails, we should still register a ref
source.
Also add an extra test for a reference between two typed properties,
as this used to be handled incorrectly earlier.
Only register the slot for adding ref sources later if we didn't
immediately register one. Also avoids leaking a ref source if
it is added early and the assignment fails.
Fixes oss-fuzz #27628.
Ilija Tovilo