Skip to content

RFC: Improve unserialize() error handling #9425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

RFC: Improve unserialize() error handling #9425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
13173ef
Add UnserializationFailedException
TimWolla Aug 25, 2022
a6cd0ac
Wrap all Exceptions thrown during `unserialize()` in UnserializationF…
TimWolla Aug 25, 2022
b117209
Increase error for garbage passed to unserialize() from Notice to War…
TimWolla Aug 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ext/standard/basic_functions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,8 @@ PHP_MINIT_FUNCTION(basic) /* {{{ */
php_ce_incomplete_class = register_class___PHP_Incomplete_Class();
php_register_incomplete_class_handlers();

php_var_ce_UnserializationFailedException = register_class_UnserializationFailedException(zend_ce_exception);

assertion_error_ce = register_class_AssertionError(zend_ce_error);

#ifdef ENABLE_TEST_CLASS
Expand Down
7 changes: 7 additions & 0 deletions ext/standard/basic_functions.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2849,6 +2849,13 @@ function memory_get_peak_usage(bool $real_usage = false): int {}

function memory_reset_peak_usage(): void {}

/**
* @strict-properties
*/
class UnserializationFailedException extends \Exception
{
}

/* versioning.c */

/** @compile-time-eval */
Expand Down
18 changes: 17 additions & 1 deletion ext/standard/basic_functions_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions ext/standard/php_var.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

PHP_MINIT_FUNCTION(var);

extern PHPAPI zend_class_entry *php_var_ce_UnserializationFailedException;

PHPAPI void php_var_dump(zval *struc, int level);
PHPAPI void php_var_export(zval *struc, int level);
PHPAPI void php_var_export_ex(zval *struc, int level, smart_str *buf);
Expand Down
8 changes: 7 additions & 1 deletion ext/standard/var.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ struct php_serialize_data {
uint32_t n;
};

PHPAPI zend_class_entry *php_var_ce_UnserializationFailedException;

#define COMMON (is_ref ? "&" : "")

static void php_array_element_dump(zval *zv, zend_ulong index, zend_string *key, int level) /* {{{ */
Expand Down Expand Up @@ -1399,7 +1401,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
}
if (!php_var_unserialize(retval, &p, p + buf_len, &var_hash)) {
if (!EG(exception)) {
php_error_docref(NULL, E_NOTICE, "Error at offset " ZEND_LONG_FMT " of %zd bytes",
php_error_docref(NULL, E_WARNING, "Error at offset " ZEND_LONG_FMT " of %zd bytes",
(zend_long)((char*)p - buf), buf_len);
}
if (BG(unserialize).level <= 1) {
Expand All @@ -1413,6 +1415,10 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
gc_check_possible_root(ref);
}

if (EG(exception)) {
zend_throw_exception_ex(php_var_ce_UnserializationFailedException, 0, "An Exception was thrown during unserialization");
}

cleanup:
if (class_hash) {
zend_hash_destroy(class_hash);
Expand Down
4 changes: 4 additions & 0 deletions ext/standard/var_unserializer.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ PHPAPI void var_destroy(php_unserialize_data_t *var_hashx)
delayed_call_failed = 1;
GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED);
}
if (EG(exception)) {
zend_throw_exception_ex(php_var_ce_UnserializationFailedException, 0, "An Exception was thrown in %s::__wakeup()", ZSTR_VAL(fci.object->ce->name));
}
BG(serialize_lock)--;

zval_ptr_dtor(&retval);
Expand All @@ -296,6 +299,7 @@ PHPAPI void var_destroy(php_unserialize_data_t *var_hashx)
Z_OBJCE_P(zv)->__unserialize, Z_OBJ_P(zv), NULL, &param);
if (EG(exception)) {
delayed_call_failed = 1;
zend_throw_exception_ex(php_var_ce_UnserializationFailedException, 0, "An Exception was thrown in %s::__unserialize()", ZSTR_VAL(Z_OBJ_P(zv)->ce->name));
GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED);
}
BG(serialize_lock)--;
Expand Down