Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit 727e26f

Browse files
committed
Fix #97836 and #81705: Segfault / type confusion in concat_function
The following sequence of actions was happening which caused a null pointer dereference: 1. debug_backtrace() returns an array 2. The concatenation to $c will transform the array to a string via `zval_get_string_func` for op2 and output a warning. Note that zval op1 is of type string due to the first do-while sequence. 3. The warning of an implicit "array to string conversion" triggers the ob_start callback to run. This code transform $c (==op1) to a long. 4. The code below the 2 do-while sequences assume that both op1 and op2 are strings, but this is no longer the case. A dereference of the string will therefore result in a null pointer dereference. The solution used here is to work with the zend_string directly instead of with the ops. For the tests: Co-authored-by: changochen1@gmail.com Co-authored-by: cmbecker69@gmx.de Co-authored-by: yukik@risec.co.jp Closes GH-10049.
1 parent 7914b8c commit 727e26f

8 files changed

+198
-40
lines changed

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ PHP NEWS
3737
index). (ColinHDev)
3838
. Fix bug GH-8846 (Implement delayed early binding for classes without
3939
parents). (ilutov)
40+
. Fix bug #79836 (Segfault in concat_function). (nielsdos)
41+
. Fix bug #81705 (type confusion/UAF on set_error_handler with concat
42+
operation). (nielsdos)
4043

4144
- Date:
4245
. Implement More Appropriate Date/Time Exceptions RFC. (Derick)

‎Zend/tests/bug79836.phpt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
Bug #79836 (Segfault in concat_function)
3+
--INI--
4+
opcache.optimization_level = 0x7FFEBFFF & ~0x400
5+
--FILE--
6+
<?php
7+
$counter = 0;
8+
ob_start(function ($buffer) use (&$c, &$counter) {
9+
$c = 0;
10+
++$counter;
11+
}, 1);
12+
$c .= [];
13+
$c .= [];
14+
ob_end_clean();
15+
echo $counter . "\n";
16+
?>
17+
--EXPECT--
18+
3

‎Zend/tests/bug79836_1.phpt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
Bug #79836 (Segfault in concat_function)
3+
--INI--
4+
opcache.optimization_level = 0x7FFEBFFF & ~0x400
5+
--FILE--
6+
<?php
7+
$x = 'non-empty';
8+
ob_start(function () use (&$c) {
9+
$c = 0;
10+
}, 1);
11+
$c = [];
12+
$x = $c . $x;
13+
$x = $c . $x;
14+
ob_end_clean();
15+
echo "Done\n";
16+
?>
17+
--EXPECT--
18+
Done

‎Zend/tests/bug79836_2.phpt‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
Bug #79836 (Segfault in concat_function)
3+
--FILE--
4+
<?php
5+
$c = str_repeat("abcd", 10);
6+
7+
ob_start(function () use (&$c) {
8+
$c = 0;
9+
}, 1);
10+
11+
class X {
12+
function __toString() {
13+
echo "a";
14+
return "abc";
15+
}
16+
}
17+
18+
$xxx = new X;
19+
20+
$x = $c . $xxx;
21+
ob_end_clean();
22+
echo $x . "\n";
23+
?>
24+
--EXPECT--
25+
abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabc

‎Zend/tests/bug81705.phpt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
Bug #81705 (type confusion/UAF on set_error_handler with concat operation)
3+
--FILE--
4+
<?php
5+
6+
$arr = [0];
7+
$my_var = str_repeat("a", 1);
8+
set_error_handler(
9+
function() use(&$my_var) {
10+
echo("error\n");
11+
$my_var = 0x123;
12+
}
13+
);
14+
$my_var .= $GLOBALS["arr"];
15+
var_dump($my_var);
16+
?>
17+
--EXPECT--
18+
error
19+
string(6) "aArray"
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
Test concatenating a class instance that has __toString with itself that uses a non-interned string
3+
--FILE--
4+
<?php
5+
$global_non_interned_string = str_repeat("a", 3);
6+
7+
class Test {
8+
public function __toString() {
9+
global $global_non_interned_string;
10+
return $global_non_interned_string;
11+
}
12+
}
13+
14+
$test1 = new Test;
15+
$test2 = new Test;
16+
$test1 .= $test2;
17+
18+
echo $test1 . "\n";
19+
?>
20+
--EXPECT--
21+
aaaaaa
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
Test concatenating a class instance that has __toString with itself
3+
--FILE--
4+
<?php
5+
class Tmp {
6+
public function __toString() {
7+
return "abc";
8+
}
9+
}
10+
11+
$tmp = new Tmp;
12+
$tmp .= $tmp;
13+
echo $tmp . "\n";
14+
?>
15+
--EXPECT--
16+
abcabc

‎Zend/zend_operators.c‎

Lines changed: 78 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1940,108 +1940,146 @@ ZEND_API zend_result ZEND_FASTCALL shift_right_function(zval *result, zval *op1,
19401940
ZEND_API zend_result ZEND_FASTCALL concat_function(zval *result, zval *op1, zval *op2) /* {{{ */
19411941
{
19421942
zval *orig_op1 = op1;
1943-
zval op1_copy, op2_copy;
1944-
1945-
ZVAL_UNDEF(&op1_copy);
1946-
ZVAL_UNDEF(&op2_copy);
1943+
zend_string *op1_string, *op2_string;
1944+
bool free_op1_string = false;
1945+
bool free_op2_string = false;
19471946

19481947
do {
1949-
if (UNEXPECTED(Z_TYPE_P(op1) != IS_STRING)) {
1948+
if (EXPECTED(Z_TYPE_P(op1) == IS_STRING)) {
1949+
op1_string = Z_STR_P(op1);
1950+
} else {
19501951
if (Z_ISREF_P(op1)) {
19511952
op1 = Z_REFVAL_P(op1);
1952-
if (Z_TYPE_P(op1) == IS_STRING) break;
1953+
if (Z_TYPE_P(op1) == IS_STRING) {
1954+
op1_string = Z_STR_P(op1);
1955+
break;
1956+
}
19531957
}
19541958
ZEND_TRY_BINARY_OBJECT_OPERATION(ZEND_CONCAT);
1955-
ZVAL_STR(&op1_copy, zval_get_string_func(op1));
1959+
op1_string=zval_get_string_func(op1);
19561960
if (UNEXPECTED(EG(exception))) {
1957-
zval_ptr_dtor_str(&op1_copy);
1961+
zend_string_release(op1_string);
19581962
if (orig_op1 != result) {
19591963
ZVAL_UNDEF(result);
19601964
}
19611965
return FAILURE;
19621966
}
1967+
free_op1_string = true;
19631968
if (result == op1) {
19641969
if (UNEXPECTED(op1 == op2)) {
1965-
op2 = &op1_copy;
1970+
op2_string = op1_string;
1971+
goto has_op2_string;
19661972
}
19671973
}
1968-
op1 = &op1_copy;
19691974
}
19701975
} while (0);
19711976
do {
1972-
if (UNEXPECTED(Z_TYPE_P(op2) != IS_STRING)) {
1973-
if (Z_ISREF_P(op2)) {
1974-
op2 = Z_REFVAL_P(op2);
1975-
if (Z_TYPE_P(op2) == IS_STRING) break;
1976-
}
1977+
if (EXPECTED(Z_TYPE_P(op2) == IS_STRING)) {
1978+
op2_string = Z_STR_P(op2);
1979+
} else {
1980+
if (Z_ISREF_P(op2)) {
1981+
op2 = Z_REFVAL_P(op2);
1982+
if (Z_TYPE_P(op2) == IS_STRING) {
1983+
op2_string = Z_STR_P(op2);
1984+
break;
1985+
}
1986+
}
1987+
/* hold an additional reference because a userland function could free this */
1988+
if (!free_op1_string) {
1989+
op1_string = zend_string_copy(op1_string);
1990+
free_op1_string = true;
1991+
}
19771992
ZEND_TRY_BINARY_OP2_OBJECT_OPERATION(ZEND_CONCAT);
1978-
ZVAL_STR(&op2_copy, zval_get_string_func(op2));
1993+
op2_string=zval_get_string_func(op2);
19791994
if (UNEXPECTED(EG(exception))) {
1980-
zval_ptr_dtor_str(&op1_copy);
1981-
zval_ptr_dtor_str(&op2_copy);
1995+
zend_string_release(op1_string);
1996+
zend_string_release(op2_string);
19821997
if (orig_op1 != result) {
19831998
ZVAL_UNDEF(result);
19841999
}
19852000
return FAILURE;
19862001
}
1987-
op2 = &op2_copy;
2002+
free_op2_string = true;
19882003
}
19892004
} while (0);
19902005

1991-
if (UNEXPECTED(Z_STRLEN_P(op1) == 0)) {
1992-
if (EXPECTED(result != op2)) {
2006+
has_op2_string:;
2007+
if (UNEXPECTED(ZSTR_LEN(op1_string) == 0)) {
2008+
if (EXPECTED(free_op2_string || result != op2)) {
19932009
if (result == orig_op1) {
19942010
i_zval_ptr_dtor(result);
19952011
}
1996-
ZVAL_COPY(result, op2);
2012+
if (free_op2_string) {
2013+
/* transfer ownership of op2_string */
2014+
ZVAL_STR(result, op2_string);
2015+
free_op2_string = false;
2016+
} else {
2017+
ZVAL_STR_COPY(result, op2_string);
2018+
}
19972019
}
1998-
} else if (UNEXPECTED(Z_STRLEN_P(op2) == 0)) {
1999-
if (EXPECTED(result != op1)) {
2020+
} else if (UNEXPECTED(ZSTR_LEN(op2_string) == 0)) {
2021+
if (EXPECTED(free_op1_string||result != op1)) {
20002022
if (result == orig_op1) {
20012023
i_zval_ptr_dtor(result);
20022024
}
2003-
ZVAL_COPY(result, op1);
2025+
if (free_op1_string) {
2026+
/* transfer ownership of op1_string */
2027+
ZVAL_STR(result, op1_string);
2028+
free_op1_string = false;
2029+
} else {
2030+
ZVAL_STR_COPY(result, op1_string);
2031+
}
20042032
}
20052033
} else {
2006-
size_t op1_len = Z_STRLEN_P(op1);
2007-
size_t op2_len = Z_STRLEN_P(op2);
2034+
size_t op1_len = ZSTR_LEN(op1_string);
2035+
size_t op2_len = ZSTR_LEN(op2_string);
20082036
size_t result_len = op1_len + op2_len;
20092037
zend_string *result_str;
2010-
uint32_t flags = ZSTR_GET_COPYABLE_CONCAT_PROPERTIES_BOTH(Z_STR_P(op1), Z_STR_P(op2));
2038+
uint32_t flags = ZSTR_GET_COPYABLE_CONCAT_PROPERTIES_BOTH(op1_string, op2_string);
20112039

20122040
if (UNEXPECTED(op1_len > ZSTR_MAX_LEN - op2_len)) {
2041+
if (free_op1_string) zend_string_release(op1_string);
2042+
if (free_op2_string) zend_string_release(op2_string);
20132043
zend_throw_error(NULL, "String size overflow");
2014-
zval_ptr_dtor_str(&op1_copy);
2015-
zval_ptr_dtor_str(&op2_copy);
20162044
if (orig_op1 != result) {
20172045
ZVAL_UNDEF(result);
20182046
}
20192047
return FAILURE;
20202048
}
20212049

2022-
if (result == op1 && Z_REFCOUNTED_P(result)) {
2050+
if (result == op1) {
2051+
if (free_op1_string) {
2052+
/* op1_string will be used as the result, so we should not free it */
2053+
i_zval_ptr_dtor(result);
2054+
free_op1_string = false;
2055+
}
20232056
/* special case, perform operations on result */
2024-
result_str = zend_string_extend(Z_STR_P(result), result_len, 0);
2057+
result_str = zend_string_extend(op1_string, result_len, 0);
2058+
/* account for the case where result_str == op1_string == op2_string and the realloc is done */
2059+
if (op1_string == op2_string) {
2060+
if (free_op2_string) {
2061+
zend_string_release(op2_string);
2062+
free_op2_string = false;
2063+
}
2064+
op2_string = result_str;
2065+
}
20252066
} else {
20262067
result_str = zend_string_alloc(result_len, 0);
2027-
memcpy(ZSTR_VAL(result_str), Z_STRVAL_P(op1), op1_len);
2068+
memcpy(ZSTR_VAL(result_str), ZSTR_VAL(op1_string), op1_len);
20282069
if (result == orig_op1) {
20292070
i_zval_ptr_dtor(result);
20302071
}
20312072
}
20322073
GC_ADD_FLAGS(result_str, flags);
20332074

2034-
/* This has to happen first to account for the cases where result == op1 == op2 and
2035-
* the realloc is done. In this case this line will also update Z_STRVAL_P(op2) to
2036-
* point to the new string. The first op2_len bytes of result will still be the same. */
20372075
ZVAL_NEW_STR(result, result_str);
2038-
2039-
memcpy(ZSTR_VAL(result_str) + op1_len, Z_STRVAL_P(op2), op2_len);
2076+
memcpy(ZSTR_VAL(result_str) + op1_len, ZSTR_VAL(op2_string), op2_len);
20402077
ZSTR_VAL(result_str)[result_len] = '0円';
20412078
}
20422079

2043-
zval_ptr_dtor_str(&op1_copy);
2044-
zval_ptr_dtor_str(&op2_copy);
2080+
if (free_op1_string) zend_string_release(op1_string);
2081+
if (free_op2_string) zend_string_release(op2_string);
2082+
20452083
return SUCCESS;
20462084
}
20472085
/* }}} */

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /