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: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: danack@php.net
New email:
PHP Version: OS:

 

 [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

Pull Requests

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-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 13:01:29 2024 UTC