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: master 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: https://www.theverge.com/users/ketoplanet2121).php.net https://docs.google.com/forms/d/e/1FAIpQLSed4L-W5rcs7dhIZBZw61FL5Vqo4mAs1OAu62udWU1czVBZ6g/viewform).php.net Previous Comments: ------------------------------------------------------------------------ [2023-01-03 10:50:26] robertnldspj11 at gmail dot com A commitment of appreciation is all together for the update and quick reply. Bet upon your string. A commitment of appreciation is all together for making it. Appreciative for sharing such mind blowing information. (https://www.myeclass.me/)github.com ------------------------------------------------------------------------ [2022-12-22 11:08:27] janettabloomquist at gmail dot com concat_function, however, implicitly assumes the variables op1 and op2 are always strings, and thus type confusion occurs as a result. UAF due to php_filter_float() failing for ints (CVE-2021-21708) ... (Potential type confusion in unixtojd() parameter parsing). (https://www.myhealthonline.biz/)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