Skip to content

Commit 2ad0b5c

Browse files
committed
Fix GH-19792: SCCP causes UAF for return value if both warning and exception are triggered
If an exception _and_ a warning (or deprecation) is emitted, then the result is destroyed twice. Use an `else if` to prevent this. This is tested via zend_test because the deprecation that triggered the original reproducer may disappear in the future. Closes GH-19793.
1 parent d30ec1d commit 2ad0b5c

File tree

6 files changed

+47
-4
lines changed

6 files changed

+47
-4
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ PHP NEWS
66
. Fixed bug GH-19765 (object_properties_load() bypasses readonly property
77
checks). (timwolla)
88
. Fixed hard_timeout with --enable-zend-max-execution-timers. (Appla)
9+
. Fixed bug GH-19792 (SCCP causes UAF for return value if both warning and
10+
exception are triggered). (nielsdos)
911

1012
- Standard:
1113
. Fixed bug GH-12265 (Cloning an object breaks serialization recursion).

Zend/Optimizer/sccp.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -842,9 +842,7 @@ static inline zend_result ct_eval_func_call(
842842
zval_ptr_dtor(result);
843843
zend_clear_exception();
844844
retval = FAILURE;
845-
}
846-
847-
if (EG(capture_warnings_during_sccp) > 1) {
845+
} else if (EG(capture_warnings_during_sccp) > 1) {
848846
zval_ptr_dtor(result);
849847
retval = FAILURE;
850848
}

ext/opcache/tests/opt/gh19792.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-19792 (SCCP causes UAF for return value if both warning and exception are triggered)
3+
--EXTENSIONS--
4+
opcache
5+
zend_test
6+
--INI--
7+
opcache.enable=1
8+
opcache.enable_cli=1
9+
opcache.optimization_level=-1
10+
--FILE--
11+
<?php
12+
13+
function foo()
14+
{
15+
return \zend_test_gh19792();
16+
}
17+
18+
try {
19+
foo();
20+
} catch (Error $e) {
21+
echo $e->getMessage(), "\n";
22+
}
23+
24+
?>
25+
--EXPECTF--
26+
Warning: a warning in %s on line %d
27+
an exception

ext/zend_test/test.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,3 +1528,12 @@ static PHP_FUNCTION(zend_test_gh18756)
15281528
zend_mm_gc(heap);
15291529
zend_mm_shutdown(heap, true, false);
15301530
}
1531+
1532+
static PHP_FUNCTION(zend_test_gh19792)
1533+
{
1534+
ZEND_PARSE_PARAMETERS_NONE();
1535+
1536+
RETVAL_STRING("this is a non-interned string");
1537+
zend_error(E_WARNING, "a warning");
1538+
zend_throw_error(NULL, "an exception");
1539+
}

ext/zend_test/test.stub.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@ function zend_test_is_zend_ptr(int $addr): bool {}
264264
function zend_test_log_err_debug(string $str): void {}
265265

266266
function zend_test_gh18756(): void {}
267+
268+
/** @compile-time-eval */
269+
function zend_test_gh19792(): void {}
267270
}
268271

269272
namespace ZendTestNS {

ext/zend_test/test_arginfo.h

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)