diff --git a/NEWS b/NEWS index a988b783f6ee2..8b74b47abbee9 100644 --- a/NEWS +++ b/NEWS @@ -124,6 +124,8 @@ PHP NEWS - Standard: . E_NOTICEs emitted by unserialize() have been promoted to E_WARNING. (timwolla) + . unserialize() now emits a new E_WARNING if the input contains unconsumed + bytes. (timwolla) . Make array_pad's $length warning less confusing. (nielsdos) . E_WARNING emitted by strtok in the caase both arguments are not provided when starting tokenisation. (David Carlier) diff --git a/UPGRADING b/UPGRADING index cdada9e9b3273..4cbf5133b8c01 100644 --- a/UPGRADING +++ b/UPGRADING @@ -87,8 +87,11 @@ PHP 8.3 UPGRADE NOTES (Alex Dowad) - Standard: - . E_NOTICEs emitted by unserialized() have been promoted to E_WARNING. + . E_NOTICEs emitted by unserialize() have been promoted to E_WARNING. RFC: https://wiki.php.net/rfc/improve_unserialize_error_handling + . unserialize() now emits a new E_WARNING if the input contains unconsumed + bytes. + RFC: https://wiki.php.net/rfc/unserialize_warn_on_trailing_data . array_pad() is now only limited by the maximum number of elements an array can have. Before, it was only possible to add at most 1048576 elements at a time. diff --git a/ext/standard/tests/serialize/typed_property_ref_overwrite.phpt b/ext/standard/tests/serialize/typed_property_ref_overwrite.phpt index 148c66b76c741..9b68e50783795 100644 --- a/ext/standard/tests/serialize/typed_property_ref_overwrite.phpt +++ b/ext/standard/tests/serialize/typed_property_ref_overwrite.phpt @@ -7,7 +7,7 @@ class Test { public ?object $prop; } $s = <<<'STR' -O:4:"Test":2:{s:4:"prop";R:1;s:4:"prop";N;}} +O:4:"Test":2:{s:4:"prop";R:1;s:4:"prop";N;} STR; var_dump(unserialize($s)); diff --git a/ext/standard/tests/serialize/unserialize_extra_data_001.phpt b/ext/standard/tests/serialize/unserialize_extra_data_001.phpt new file mode 100644 index 0000000000000..421fb3428b3a7 --- /dev/null +++ b/ext/standard/tests/serialize/unserialize_extra_data_001.phpt @@ -0,0 +1,26 @@ +--TEST-- +Test unserialize() with extra data at the end of a valid value +--FILE-- + +--EXPECTF-- +Warning: unserialize(): Extra data starting at offset 4 of 8 bytes in %s on line %d +int(5) + +Warning: unserialize(): Extra data starting at offset 2 of 6 bytes in %s on line %d +NULL + +Warning: unserialize(): Extra data starting at offset 4 of 8 bytes in %s on line %d +bool(true) + +Warning: unserialize(): Extra data starting at offset 20 of 24 bytes in %s on line %d +array(1) { + ["foo"]=> + bool(true) +} diff --git a/ext/standard/tests/serialize/unserialize_extra_data_002.phpt b/ext/standard/tests/serialize/unserialize_extra_data_002.phpt new file mode 100644 index 0000000000000..31e6a227f7112 --- /dev/null +++ b/ext/standard/tests/serialize/unserialize_extra_data_002.phpt @@ -0,0 +1,42 @@ +--TEST-- +Test unserialize() with extra data at the end of a valid value with nested unserialize +--FILE-- +foo = unserialize($foo['bar']); + } + + public function __serialize(): array + { + return [ + 'bar' => serialize($this->foo) . 'garbage', + ]; + } +} + +$f = new Foo; +$f->foo = ['a', 'b', 'c']; + +var_dump(unserialize(serialize($f) . 'garbage')); + +?> +--EXPECTF-- +Warning: unserialize(): Extra data starting at offset 81 of 88 bytes in %s on line %d + +Warning: unserialize(): Extra data starting at offset 42 of 49 bytes in %s on line %d +object(Foo)#2 (1) { + ["foo"]=> + array(3) { + [0]=> + string(1) "a" + [1]=> + string(1) "b" + [2]=> + string(1) "c" + } +} diff --git a/ext/standard/tests/serialize/unserialize_extra_data_003.phpt b/ext/standard/tests/serialize/unserialize_extra_data_003.phpt new file mode 100644 index 0000000000000..93f2c49a56a0c --- /dev/null +++ b/ext/standard/tests/serialize/unserialize_extra_data_003.phpt @@ -0,0 +1,42 @@ +--TEST-- +Test unserialize() with extra data at the end of a valid value with Serializable +--FILE-- +foo = unserialize($foo); + } + + public function serialize(): string + { + return serialize($this->foo) . 'garbage'; + } +} + +$f = new Foo; +$f->foo = ['a', 'b', 'c']; + +var_dump(unserialize(serialize($f) . 'garbage')); + +?> +--EXPECTF-- +Deprecated: Foo implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in %s on line %d + +Warning: unserialize(): Extra data starting at offset 42 of 49 bytes in %s on line %d + +Warning: unserialize(): Extra data starting at offset 64 of 71 bytes in %s on line %d +object(Foo)#2 (1) { + ["foo"]=> + array(3) { + [0]=> + string(1) "a" + [1]=> + string(1) "b" + [2]=> + string(1) "c" + } +} diff --git a/ext/standard/var.c b/ext/standard/var.c index e8145ab227141..784903616724a 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -1406,11 +1406,19 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co zval_ptr_dtor(return_value); } RETVAL_FALSE; - } else if (BG(unserialize).level > 1) { - ZVAL_COPY(return_value, retval); - } else if (Z_REFCOUNTED_P(return_value)) { - zend_refcounted *ref = Z_COUNTED_P(return_value); - gc_check_possible_root(ref); + } else { + if ((char*)p < buf + buf_len) { + if (!EG(exception)) { + php_error_docref(NULL, E_WARNING, "Extra data starting at offset " ZEND_LONG_FMT " of %zd bytes", + (zend_long)((char*)p - buf), buf_len); + } + } + if (BG(unserialize).level > 1) { + ZVAL_COPY(return_value, retval); + } else if (Z_REFCOUNTED_P(return_value)) { + zend_refcounted *ref = Z_COUNTED_P(return_value); + gc_check_possible_root(ref); + } } cleanup: