Bug #64993 [Opn->Csd]: [patch] PDO::query() memory leak and reference problem if error

Edit report at https://bugs.php.net/bug.php?id=64993&edit=1 ID: 64993 Updated by: nikic@php.net Reported by: rgagnon24 at gmail dot com Summary: [patch] PDO::query() memory leak and reference problem if error -Status: Open +Status: Closed Type: Bug Package: PDO MySQL Operating System: Any PHP Version: 5.4.16 -Assigned To: +Assigned To: nikic Block user comment: N Private report: N New Comment: This should be fixed by https://github.com/php/php-src/commit/6202e2860d84b7415a9da613e77ac800d64a1a51 in PHP 7. Previous Comments: ------------------------------------------------------------------------ [2013-11-01 07:03:17] rgagnon24 at gmail dot com Nice catch, yogaki. However, is there any need to assign return_value to dbh->query_stmt_zval if FALSE is going to be sent back anyhow? By the time execution is at either side of that "else", we are in an error condition. The positive return happens just above the "/* something broke */" comment inside the "if (ret) {" ------------------------------------------------------------------------ [2013-11-01 06:46:21] yohgaki@php.net Your patch may solve your problem, but it may cause other problem. /* something broke */ dbh->query_stmt = stmt; dbh->query_stmt_zval = *return_value; PDO_HANDLE_STMT_ERR(); } else { PDO_HANDLE_DBH_ERR(); zval_dtor(return_value); } RETURN_FALSE; Since return_value is assigned to dbh->query_stmt_zval = *return_value; it seems we cannot free return_value. ------------------------------------------------------------------------ [2013-06-14 05:13:59] rgagnon24 at gmail dot com About the "security" type of bug filed. I think I missed the "bug" option by one in the selector and got that one by accident. I did want to mention it could help become a security/DOS problem with the bug, but not record it as a security issue. I am testing now to see if with and without the bug if the "max_links" ini settings are still obeyed--which might make it a problem at that point as the bug would allow someone to workaround an admin setting. For now this does not appear to be the case though. In other news..... This patch appears to also resolve the problem in bug 64549 that I also reported a while back. Possibly the correct free'ing of the resources here eliminated the conditions that cause the error on that bug. I have seen a couple of other bugs that are PDO related that I am going back to test with this patch to see if they may also be resolved as well. ------------------------------------------------------------------------ [2013-06-14 05:09:14] rgagnon24 at gmail dot com Related To: Bug #64549 ------------------------------------------------------------------------ [2013-06-10 11:40:52] johannes@php.net This is no security issues. Users who want to hold a connection open can do this without this bug, too. ------------------------------------------------------------------------ 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=64993 -- Edit this bug report at https://bugs.php.net/bug.php?id=64993&edit=1