Edit report at https://bugs.php.net/bug.php?id=81705&edit=1
ID: 81705
Comment by: neske456 at gmail dot com
Reported by: yukik at ricsec dot co dot jp
Summary: type confusion/UAF on set_error_handler with concat
operation
Status: Verified
Type: Bug
Package: Scripting Engine problem
Operating System: Linux
PHP Version: 8.0.14
Block user comment: N
Private report: N
New Comment:
The error handler must return FALSE to populate $php_errormsg
<https://www.clover-dashboard.com/>github.com
Previous Comments:
------------------------------------------------------------------------
[2022-11-29 05:00:15] fosernie at gmail dot com
Set a user-defined error handler function with the set_error_handler () function, and trigger an
error (with trigger_error ()): The set_error_handler () function sets a user-defined error handler
function.
<https://www.fragomenconnect.com/>github.com
------------------------------------------------------------------------
[2022-01-06 22:45:18] 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>
------------------------------------------------------------------------
[2022-01-06 09:42:54] 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-05 15:13:52] 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-05 04:52:32] 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.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=81705
--
Edit this bug report at https://bugs.php.net/bug.php?id=81705&edit=1