php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #68760 SQLITE segfaults if custom collator throws an exception
Submitted: 2015-01-06 20:56 UTC Modified: 2015-03-22 23:40 UTC
From: danack@php.net Assigned:
Status: Closed Package: SQLite (PECL)
PHP Version: master-Git-2015-01-06 (Git) OS: N/A
Private report: No CVE-ID:
 [2015-01-06 20:56 UTC] danack@php.net
Description:
------------
The code below segfaults on PHP 5.X. I have a patch for that which I'll submit once there is also one for PHP 7.

For PHP 7 the first zend_call_function to the function that throws the exception does not return FAILURE, which makes detecting whether the function succeeded or not be a bit tricky.

The code that has the issue is here:
http://lxr.php.net/xref/PHP_5_6/ext/sqlite3/sqlite3.c#900

Adding a debug line of:
    printf("ret is %x, failure is %x, success is %x\n", ret, FAILURE, SUCCESS);

after the zend_call_function produces the output:
    ret is 0, failure is ffffffff, success is 0


It looks like someone had similar trouble in libxml:
http://lxr.php.net/xref/PHP_TRUNK/ext/libxml/libxml.c#607

Where they are testing "if (status != SUCCESS || Z_ISUNDEF(retval)) {" to see if the zend_call_function succeeded or not.

It should be able to test if the function call succeeded by the return value from zend_call_function right?


Test script:
---------------
<?php

function oopsFunction($a, $b) {
	echo "This is inside the callback.\n";
    throw new \Exception("oops $a $b");
}

$db = new SQLite3(":memory:");
$db->exec("CREATE TABLE test (col1 string)");
$db->exec("INSERT INTO test VALUES ('a1')");
$db->exec("INSERT INTO test VALUES ('a10')");
$db->exec("INSERT INTO test VALUES ('a2')");

try {
    $db->createCollation('NATURAL_CMP', 'oopsFunction');
    $naturalSort = $db->query("SELECT col1 FROM test ORDER BY col1 COLLATE NATURAL_CMP");
    while ($row = $naturalSort->fetchArray()) {
        echo $row['col1'], "\n";
    }
    $db->close();
}
catch(\Exception $e) {
    echo "Exception: ".$e->getMessage();
}

Expected result:
----------------
zend_call_function on line 863 should return FAILURE as the function threw an exception.

Actual result:
--------------
zend_call_function returns SUCCESS even though the function threw an exception.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-01-09 07:15 UTC] krakjoe@php.net
A function call can succeed and not set a return value; it's conventional (and sensible) to check that the return value pointer has been set and or allocated before referencing it to avoid segfaults.
 [2015-01-10 16:02 UTC] laruence@php.net
where is the patch you said?
 [2015-01-10 16:02 UTC] laruence@php.net
where is the patch you said?
 [2015-01-10 19:42 UTC] danack@php.net
The patch/PR for 5.6 is here, it's pretty trivial:
https://github.com/Danack/php-src/compare/sqlite3_exceptionSegfault56?expand=1


> I have a patch for that which I'll submit once there is also one for PHP 7.

As I said, I am planning to submit it when I can also do one for PHP 7, which would require the underlying bug to be fixed. 

There is other behaviour in the sqlite code that that ought to be fixed - namely the warning that is generated on the exception is bogus.
 [2015-01-10 22:15 UTC] philip@php.net
How do we change this to a SQLite3 bug? It's not related to pecl/sqlite (currently selected package) but I don't see a way to change the Package.
 [2015-01-10 23:30 UTC] danack@php.net
philip, apparently I should have set it to "scripting engine problem" as that is the entry used for Zend internal issues. I can't see any way to change it to that now.

And yes, we're also completely missing an SQLite3 package.
 [2015-01-15 18:37 UTC] philip@php.net
Ah Danack, I see. And once a bug is designated as a "PECL" bug it cannot be changed to a non-PECL bug. That's unfortunate.
 [2015-03-04 10:52 UTC] laruence@php.net
you didn't send the PR?
 [2015-03-22 23:40 UTC] stas@php.net
-Summary: zend_call_function exception bad behaviour +Summary: SQLITE segfaults if custom collator throws an exception
 [2015-03-22 23:49 UTC] stas@php.net
Automatic comment on behalf of Danack@basereality.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=0c27a8eb61813f04c92caf578d24bb3b76eb6651
Log: Fix #68760: Fix freeing null segfault. Added test for behaviour.
 [2015-03-22 23:49 UTC] stas@php.net
-Status: Open +Status: Closed
 [2015-03-22 23:49 UTC] stas@php.net
Automatic comment on behalf of Danack@basereality.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=1ec430d4edd90ad4ac6523b23ca669253535bc44
Log: Fix #68760: Fix freeing null segfault. Added test for behaviour.
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sat Jul 22 20:01:35 2017 UTC