Bug #81705 [Com]: type confusion/UAF on set_error_handler with concat operation

Edit report at https://bugs.php.net/bug.php?id=81705&edit=1 ID: 81705 Comment by: hazelmarissa1 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: This article is really amazing. Thanks for the sharing. <https://www.myhdfs.net/>github.com Previous Comments: ------------------------------------------------------------------------ [2022-12-02 05:19:28] neske456 at gmail dot com The error handler must return FALSE to populate $php_errormsg <https://www.clover-dashboard.com/>github.com ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ 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