Bug #80284 [Com]: Potential issue in win32/signal.c: Return Value Not Checked from Function Call

Edit report at https://bugs.php.net/bug.php?id=80284&edit=1 ID: 80284 Comment by: StephendfgBaker at outlook dot com Reported by: sagpant at microsoft dot com Summary: Potential issue in win32/signal.c: Return Value Not Checked from Function Call Status: Open Type: Bug Package: *General Issues PHP Version: 7.4.11 Block user comment: N Private report: N New Comment: (https://github.com)(https://www.doglikesbest.com/) That was so amazing. Previous Comments: ------------------------------------------------------------------------ [2020-11-13 18:05:39] requinix@php.net Related To: Bug #80364 ------------------------------------------------------------------------ [2020-11-09 12:50:49] cmb@php.net The following pull request has been associated: Patch Name: Fix #80284: Return Value Not Checked from Function Call On GitHub: https://github.com/php/php-src/pull/6412 Patch: https://github.com/php/php-src/pull/6412.patch ------------------------------------------------------------------------ [2020-10-27 15:39:31] sagpant at microsoft dot com I agree with you that the retval initialization is taken care by the initialization function itself. So, as you pointed out that it would only make sense to check the value of retval as IS_UNDEF. The reason our system flagged this issue is because it found different instances of this function call where the return value from the function call was checked for SUCCESS/FAILURE. Since our system learns from the repo codebase itself, in this case php, it found inconsistent usage of this function call_user_function. This function is called for a total of 39 times and out these 39 times, return value is checked at 30 instances and at 2 separate instances, argument is initialized before making the function call. ------------------------------------------------------------------------ [2020-10-26 17:47:02] cmb@php.net call_user_function() calls _call_user_function_ex()[1] which in turn calls zend_call_function()[2]. The latter defines the return value (IS_UNDEF) before the function could fail. So if call_user_function() fails, the following zval_ptr_dtor() would practically be a NOP. As such I don't see a security issue here. I also don't see that there is any special code path that should be exercised if call_user_function() fails here, except maybe for asserting that retval is indeed IS_UNDEF. [1] <https://github.com/php/php-src/blob/php-7.4.11/Zend/zend_execute_API.c#L633> [2] <https://github.com/php/php-src/blob/php-7.4.11/Zend/zend_execute_API.c#L649> ------------------------------------------------------------------------ [2020-10-26 17:39:32] stas@php.net Could you please explain what is the security issue here? Code readability is not a security issue per se. ------------------------------------------------------------------------ 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=80284 -- Edit this bug report at https://bugs.php.net/bug.php?id=80284&edit=1