Skip to content

Fix leak when overriding Exception get hooks #25177

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

Open
iluuu1994 wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
Open

Fix leak when overriding Exception get hooks #25177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 42 additions & 0 deletions Zend/tests/exception_hooks_leak.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
--TEST--
Overridden Exception get hooks don't leak
--CREDITS--
Saad Fellahi (TarPeg007)
--FILE--
<?php

class E extends Exception {
public $code = 0 { get => $this->code . '!'; }
public string $file { get => $this->file . '!'; }
public $message { get => $this->message . '!'; }
}

$e = new E('Foo');
var_dump($e->getCode());
var_dump($e->getFile());
var_dump($e->getMessage());
var_dump($e->getTrace());
echo $e, "\n";

class E2 extends Exception {
public $code = 0 {
&get {
$value = $this->code . '!';
return $value;
}
}
}
$e2 = new E2('Foo');
var_dump($e2->getCode());

?>
--EXPECTF--
string(2) "0!"
string(%d) "%sexception_hooks_leak.php!"
string(4) "Foo!"
array(0) {
}
E: Foo! in %s:%d
Stack trace:
#0 {main}
string(2) "0!"
109 changes: 64 additions & 45 deletions Zend/zend_exceptions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,42 @@ ZEND_API zend_class_entry *zend_get_exception_base(zend_object *object) /* {{{ *
}
/* }}} */

static zend_always_inline zend_string *get_string_property(zval *obj, zend_string *prop_name, bool silent)
{
zval rv;
zval *prop = zend_read_property_ex(i_get_exception_base(Z_OBJ_P(obj)), Z_OBJ_P(obj), prop_name, silent, &rv);
zend_string *result = zval_get_string(prop);
if (prop == &rv) {
zval_ptr_dtor(&rv);
}
return result;
}

static zend_always_inline zend_long get_long_property(zval *obj, zend_string *prop_name, bool silent)
{
zval rv;
zval *prop = zend_read_property_ex(i_get_exception_base(Z_OBJ_P(obj)), Z_OBJ_P(obj), prop_name, silent, &rv);
zend_long result = zval_get_long(prop);
if (prop == &rv) {
zval_ptr_dtor(&rv);
}
return result;
}

static zend_always_inline zval *get_previous_exception(zend_object *obj, bool silent)
{
zval rv;
zval *exception = zend_read_property_ex(i_get_exception_base(obj), obj, ZSTR_KNOWN(ZEND_STR_PREVIOUS), silent, &rv);
/* $previous is private and cannot be computed. */
ZEND_ASSERT(exception != &rv);
ZVAL_DEREF(exception);
return exception;
}

void zend_exception_set_previous(zend_object *exception, zend_object *add_previous) /* {{{ */
{
zval *previous, *ancestor, *ex;
zval pv, zv, rv;
zend_class_entry *base_ce;
zval pv, zv;

if (!exception || !add_previous) {
return;
Expand All @@ -117,21 +148,17 @@ void zend_exception_set_previous(zend_object *exception, zend_object *add_previo
ZVAL_OBJ(&zv, exception);
ex = &zv;
do {
ancestor = zend_read_property_ex(i_get_exception_base(add_previous), add_previous, ZSTR_KNOWN(ZEND_STR_PREVIOUS), 1, &rv);
ZVAL_DEREF(ancestor);
ancestor = get_previous_exception(add_previous, /* silent */ true);
while (Z_TYPE_P(ancestor) == IS_OBJECT) {
if (Z_OBJ_P(ancestor) == Z_OBJ_P(ex)) {
OBJ_RELEASE(add_previous);
return;
}
ancestor = zend_read_property_ex(i_get_exception_base(Z_OBJ_P(ancestor)), Z_OBJ_P(ancestor), ZSTR_KNOWN(ZEND_STR_PREVIOUS), 1, &rv);
ZVAL_DEREF(ancestor);
ancestor = get_previous_exception(Z_OBJ_P(ancestor), /* silent */ true);
}
base_ce = i_get_exception_base(Z_OBJ_P(ex));
previous = zend_read_property_ex(base_ce, Z_OBJ_P(ex), ZSTR_KNOWN(ZEND_STR_PREVIOUS), 1, &rv);
ZVAL_DEREF(previous);
previous = get_previous_exception(Z_OBJ_P(ex), /* silent */ true);
if (Z_TYPE_P(previous) == IS_NULL) {
zend_update_property_ex(base_ce, Z_OBJ_P(ex), ZSTR_KNOWN(ZEND_STR_PREVIOUS), &pv);
zend_update_property_ex(i_get_exception_base(Z_OBJ_P(ex)), Z_OBJ_P(ex), ZSTR_KNOWN(ZEND_STR_PREVIOUS), &pv);
GC_DELREF(add_previous);
return;
}
Expand Down Expand Up @@ -436,42 +463,31 @@ ZEND_METHOD(ErrorException, __construct)

#define GET_PROPERTY(object, id) \
zend_read_property_ex(i_get_exception_base(Z_OBJ_P(object)), Z_OBJ_P(object), ZSTR_KNOWN(id), 0, &rv)
#define GET_PROPERTY_SILENT(object, id) \
zend_read_property_ex(i_get_exception_base(Z_OBJ_P(object)), Z_OBJ_P(object), ZSTR_KNOWN(id), 1, &rv)

/* {{{ Get the file in which the exception occurred */
ZEND_METHOD(Exception, getFile)
{
zval *prop, rv;

ZEND_PARSE_PARAMETERS_NONE();

prop = GET_PROPERTY(ZEND_THIS, ZEND_STR_FILE);
RETURN_STR(zval_get_string(prop));
RETURN_STR(get_string_property(ZEND_THIS, ZSTR_KNOWN(ZEND_STR_FILE), /* silent */ false));
}
/* }}} */

/* {{{ Get the line in which the exception occurred */
ZEND_METHOD(Exception, getLine)
{
zval *prop, rv;

ZEND_PARSE_PARAMETERS_NONE();

prop = GET_PROPERTY(ZEND_THIS, ZEND_STR_LINE);
RETURN_LONG(zval_get_long(prop));
RETURN_LONG(get_long_property(ZEND_THIS, ZSTR_KNOWN(ZEND_STR_LINE), /* silent */ false));
}
/* }}} */

/* {{{ Get the exception message */
ZEND_METHOD(Exception, getMessage)
{
zval *prop, rv;

ZEND_PARSE_PARAMETERS_NONE();

prop = GET_PROPERTY(ZEND_THIS, ZEND_STR_MESSAGE);
RETURN_STR(zval_get_string(prop));
RETURN_STR(get_string_property(ZEND_THIS, ZSTR_KNOWN(ZEND_STR_MESSAGE), /* silent */ false));
}
/* }}} */

Expand All @@ -483,8 +499,12 @@ ZEND_METHOD(Exception, getCode)
ZEND_PARSE_PARAMETERS_NONE();

prop = GET_PROPERTY(ZEND_THIS, ZEND_STR_CODE);
bool computed = prop == &rv;
ZVAL_DEREF(prop);
ZVAL_COPY(return_value, prop);
if (computed) {
zval_ptr_dtor(&rv);
}
}
/* }}} */

Expand All @@ -496,6 +516,8 @@ ZEND_METHOD(Exception, getTrace)
ZEND_PARSE_PARAMETERS_NONE();

prop = GET_PROPERTY(ZEND_THIS, ZEND_STR_TRACE);
/* $trace is private and cannot be computed. */
ZEND_ASSERT(prop != &rv);
ZVAL_DEREF(prop);
ZVAL_COPY(return_value, prop);
}
Expand All @@ -509,7 +531,8 @@ ZEND_METHOD(ErrorException, getSeverity)
ZEND_PARSE_PARAMETERS_NONE();

prop = GET_PROPERTY(ZEND_THIS, ZEND_STR_SEVERITY);
ZVAL_DEREF(prop);
/* $severity is typed int and is never refcounted. */
ZEND_ASSERT(prop != &rv || !Z_REFCOUNTED(rv));
ZVAL_COPY(return_value, prop);
}
/* }}} */
Expand Down Expand Up @@ -672,11 +695,9 @@ ZEND_METHOD(Exception, getTraceAsString)
/* {{{ Return previous Throwable or NULL. */
ZEND_METHOD(Exception, getPrevious)
{
zval rv;

ZEND_PARSE_PARAMETERS_NONE();

ZVAL_COPY_DEREF(return_value, GET_PROPERTY_SILENT(ZEND_THIS, ZEND_STR_PREVIOUS));
ZVAL_COPY(return_value, get_previous_exception(Z_OBJ_P(ZEND_THIS), /* silent */ true));
} /* }}} */

/* {{{ Obtain the string representation of the Exception object */
Expand All @@ -686,7 +707,7 @@ ZEND_METHOD(Exception, __toString)
zend_class_entry *base_ce;
zend_string *str;
zend_fcall_info fci;
zval rv, tmp;
zval tmp;
zend_string *fname;

ZEND_PARSE_PARAMETERS_NONE();
Expand All @@ -698,9 +719,9 @@ ZEND_METHOD(Exception, __toString)

while (exception && Z_TYPE_P(exception) == IS_OBJECT && instanceof_function(Z_OBJCE_P(exception), zend_ce_throwable)) {
zend_string *prev_str = str;
zend_string *message = zval_get_string(GET_PROPERTY(exception, ZEND_STR_MESSAGE));
zend_string *file = zval_get_string(GET_PROPERTY(exception, ZEND_STR_FILE));
zend_long line = zval_get_long(GET_PROPERTY(exception, ZEND_STR_LINE));
zend_string *message = get_string_property(exception, ZSTR_KNOWN(ZEND_STR_MESSAGE), /* silent */ false);
zend_string *file = get_string_property(exception, ZSTR_KNOWN(ZEND_STR_FILE), /* silent */ false);
zend_long line = get_long_property(exception, ZSTR_KNOWN(ZEND_STR_LINE), /* silent */ false);

fci.size = sizeof(fci);
ZVAL_STR(&fci.function_name, fname);
Expand Down Expand Up @@ -749,8 +770,7 @@ ZEND_METHOD(Exception, __toString)
zval_ptr_dtor(&trace);

Z_PROTECT_RECURSION_P(exception);
exception = GET_PROPERTY(exception, ZEND_STR_PREVIOUS);
ZVAL_DEREF(exception);
exception = get_previous_exception(Z_OBJ_P(exception), /* silent */ false);
if (Z_TYPE_P(exception) == IS_OBJECT && Z_IS_RECURSIVE_P(exception)) {
break;
}
Expand All @@ -765,8 +785,7 @@ ZEND_METHOD(Exception, __toString)
} else {
break;
}
exception = GET_PROPERTY(exception, ZEND_STR_PREVIOUS);
ZVAL_DEREF(exception);
exception = get_previous_exception(Z_OBJ_P(exception), /* silent */ false);
}

exception = ZEND_THIS;
Expand Down Expand Up @@ -930,17 +949,17 @@ static void zend_error_va(int type, zend_string *file, uint32_t lineno, const ch
/* This function doesn't return if it uses E_ERROR */
ZEND_API ZEND_COLD zend_result zend_exception_error(zend_object *ex, int severity) /* {{{ */
{
zval exception, rv;
zval exception;
zend_class_entry *ce_exception;
zend_result result = FAILURE;

ZVAL_OBJ(&exception, ex);
ce_exception = ex->ce;
EG(exception) = NULL;
if (ce_exception == zend_ce_parse_error || ce_exception == zend_ce_compile_error) {
zend_string *message = zval_get_string(GET_PROPERTY(&exception, ZEND_STR_MESSAGE));
zend_string *file = zval_get_string(GET_PROPERTY_SILENT(&exception, ZEND_STR_FILE));
zend_long line = zval_get_long(GET_PROPERTY_SILENT(&exception, ZEND_STR_LINE));
zend_string *message = get_string_property(&exception, ZSTR_KNOWN(ZEND_STR_MESSAGE), /* silent */ false);
zend_string *file = get_string_property(&exception, ZSTR_KNOWN(ZEND_STR_FILE), /* silent */ true);
zend_long line = get_long_property(&exception, ZSTR_KNOWN(ZEND_STR_LINE), /* silent */ true);
int type = (ce_exception == zend_ce_parse_error ? E_PARSE : E_COMPILE_ERROR) | E_DONT_BAIL;

zend_observer_error_notify(type, file, line, message);
Expand Down Expand Up @@ -969,8 +988,8 @@ ZEND_API ZEND_COLD zend_result zend_exception_error(zend_object *ex, int severit
ZVAL_OBJ(&zv, EG(exception));
/* do the best we can to inform about the inner exception */
if (instanceof_function(ce_exception, zend_ce_exception) || instanceof_function(ce_exception, zend_ce_error)) {
file = zval_get_string(GET_PROPERTY_SILENT(&zv, ZEND_STR_FILE));
line = zval_get_long(GET_PROPERTY_SILENT(&zv, ZEND_STR_LINE));
file = get_string_property(&zv, ZSTR_KNOWN(ZEND_STR_FILE), /* silent */ true);
line = get_long_property(&zv, ZSTR_KNOWN(ZEND_STR_LINE), /* silent */ true);
}

zend_error_va(E_WARNING, (file && ZSTR_LEN(file) > 0) ? file : NULL, line,
Expand All @@ -982,9 +1001,9 @@ ZEND_API ZEND_COLD zend_result zend_exception_error(zend_object *ex, int severit
}
}

str = zval_get_string(GET_PROPERTY_SILENT(&exception, ZEND_STR_STRING));
file = zval_get_string(GET_PROPERTY_SILENT(&exception, ZEND_STR_FILE));
line = zval_get_long(GET_PROPERTY_SILENT(&exception, ZEND_STR_LINE));
str = get_string_property(&exception, ZSTR_KNOWN(ZEND_STR_STRING), /* silent */ true);
file = get_string_property(&exception, ZSTR_KNOWN(ZEND_STR_FILE), /* silent */ true);
line = get_long_property(&exception, ZSTR_KNOWN(ZEND_STR_LINE), /* silent */ true);

zend_error_va(severity | E_DONT_BAIL,
(file && ZSTR_LEN(file) > 0) ? file : NULL, line,
Expand Down
Loading