Skip to content

Commit 7300fa5

Browse files
iliaaliliaal
committed
ext/sqlite3: reject close() from inside a callback
SQLite3::close() called from within a userland function, aggregate, collation or authorizer callback freed the registered statements and functions while sqlite3 was still executing, corrupting the active statement and crashing the request. Track callback re-entry with a per-database counter shared by all four callback kinds and throw an Error from close() while it is non-zero; the database stays usable and can be closed after the query completes. Closes GH-23650
1 parent 4e26dd0 commit 7300fa5

7 files changed

Lines changed: 212 additions & 0 deletions

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ PHP NEWS
2727
. Fixed PDOStatement::getColumnMeta() reading out of bounds for an invalid
2828
column index. (Ilia Alshanetsky)
2929

30+
- SQLite:
31+
. Fixed a crash when SQLite3::close() is called from a userland callback.
32+
(Ilia Alshanetsky)
33+
3034
- Zip:
3135
. Fixed ZipArchive::extractTo() ignoring files given in a non-list array.
3236
(David Carlier)

‎ext/sqlite3/php_sqlite3_structs.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ typedef struct _php_sqlite3_func {
5050
zend_fcall_info_cache func;
5151
zend_fcall_info_cache step;
5252
zend_fcall_info_cache fini;
53+
unsigned int *in_callback_ptr;
5354
} php_sqlite3_func;
5455

5556
/* Structure for SQLite collation function */
@@ -58,6 +59,7 @@ typedef struct _php_sqlite3_collation {
5859

5960
const char *collation_name;
6061
zend_fcall_info_cache cmp_func;
62+
unsigned int *in_callback_ptr;
6163
} php_sqlite3_collation;
6264

6365
/* Structure for SQLite Database object. */
@@ -69,6 +71,7 @@ typedef struct _php_sqlite3_db_object {
6971
zend_fcall_info_cache authorizer_fcc;
7072

7173
bool exception;
74+
unsigned int in_callback;
7275

7376
zend_llist free_list;
7477
zend_object zo;

‎ext/sqlite3/sqlite3.c‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ PHP_METHOD(SQLite3, close)
191191
}
192192

193193
if (db_obj->initialised) {
194+
if (db_obj->in_callback) {
195+
zend_throw_error(NULL, "Cannot close SQLite3 database while inside a callback");
196+
RETURN_THROWS();
197+
}
194198
zend_llist_clean(&(db_obj->free_list));
195199
if(db_obj->db) {
196200
errcode = sqlite3_close(db_obj->db);
@@ -774,13 +778,20 @@ static int sqlite3_do_callback(zend_fcall_info_cache *fcc, uint32_t argc, sqlite
774778
uint32_t fake_argc;
775779
zend_result ret = SUCCESS;
776780
php_sqlite3_agg_context *agg_context = NULL;
781+
bool bailout = false;
782+
php_sqlite3_func *cb_func = (php_sqlite3_func *)sqlite3_user_data(context);
783+
unsigned int *in_callback = cb_func ? cb_func->in_callback_ptr : NULL;
777784

778785
if (is_agg) {
779786
is_agg = 2;
780787
}
781788

782789
fake_argc = argc + is_agg;
783790

791+
if (in_callback) {
792+
(*in_callback)++;
793+
}
794+
784795
/* build up the params */
785796
if (fake_argc) {
786797
zargs = (zval *)safe_emalloc(fake_argc, sizeof(zval), 0);
@@ -823,7 +834,15 @@ static int sqlite3_do_callback(zend_fcall_info_cache *fcc, uint32_t argc, sqlite
823834
}
824835
}
825836

837+
zend_try {
826838
zend_call_known_fcc(fcc, &retval, fake_argc, zargs, /* named_params */ NULL);
839+
} zend_catch {
840+
bailout = true;
841+
} zend_end_try();
842+
843+
if (in_callback) {
844+
(*in_callback)--;
845+
}
827846

828847
/* clean up the params */
829848
if (is_agg) {
@@ -889,6 +908,9 @@ static int sqlite3_do_callback(zend_fcall_info_cache *fcc, uint32_t argc, sqlite
889908
if (!Z_ISUNDEF(retval)) {
890909
zval_ptr_dtor(&retval);
891910
}
911+
if (bailout) {
912+
zend_bailout();
913+
}
892914
return ret;
893915
}
894916
/* }}}*/
@@ -929,6 +951,7 @@ static int php_sqlite3_callback_compare(void *coll, int a_len, const void *a, in
929951
zval zargs[2];
930952
zval retval;
931953
int ret = 0;
954+
bool bailout = false;
932955

933956
// Exception occurred on previous callback. Don't attempt to call function.
934957
if (EG(exception)) {
@@ -938,10 +961,26 @@ static int php_sqlite3_callback_compare(void *coll, int a_len, const void *a, in
938961
ZVAL_STRINGL(&zargs[0], a, a_len);
939962
ZVAL_STRINGL(&zargs[1], b, b_len);
940963

964+
if (collation->in_callback_ptr) {
965+
(*collation->in_callback_ptr)++;
966+
}
967+
968+
zend_try {
941969
zend_call_known_fcc(&collation->cmp_func, &retval, /* argc */ 2, zargs, /* named_params */ NULL);
970+
} zend_catch {
971+
bailout = true;
972+
} zend_end_try();
973+
974+
if (collation->in_callback_ptr) {
975+
(*collation->in_callback_ptr)--;
976+
}
942977

943978
zval_ptr_dtor(&zargs[0]);
944979
zval_ptr_dtor(&zargs[1]);
980+
if (bailout) {
981+
zval_ptr_dtor(&retval);
982+
zend_bailout();
983+
}
945984

946985
if (EG(exception)) {
947986
ret = 0;
@@ -988,6 +1027,7 @@ PHP_METHOD(SQLite3, createFunction)
9881027
}
9891028

9901029
func = (php_sqlite3_func *)ecalloc(1, sizeof(*func));
1030+
func->in_callback_ptr = &db_obj->in_callback;
9911031

9921032
if (sqlite3_create_function(db_obj->db, sql_func, sql_func_num_args, flags | SQLITE_UTF8, func, php_sqlite3_callback_func, NULL, NULL) == SQLITE_OK) {
9931033
func->func_name = estrdup(sql_func);
@@ -1037,6 +1077,7 @@ PHP_METHOD(SQLite3, createAggregate)
10371077
}
10381078

10391079
func = (php_sqlite3_func *)ecalloc(1, sizeof(*func));
1080+
func->in_callback_ptr = &db_obj->in_callback;
10401081

10411082
if (sqlite3_create_function(db_obj->db, sql_func, sql_func_num_args, SQLITE_UTF8, func, NULL, php_sqlite3_callback_step, php_sqlite3_callback_final) == SQLITE_OK) {
10421083
func->func_name = estrdup(sql_func);
@@ -1085,6 +1126,7 @@ PHP_METHOD(SQLite3, createCollation)
10851126
}
10861127

10871128
collation = (php_sqlite3_collation *)ecalloc(1, sizeof(*collation));
1129+
collation->in_callback_ptr = &db_obj->in_callback;
10881130
if (sqlite3_create_collation(db_obj->db, collation_name, SQLITE_UTF8, collation, php_sqlite3_callback_compare) == SQLITE_OK) {
10891131
collation->collation_name = estrdup(collation_name);
10901132

@@ -2151,8 +2193,15 @@ static int php_sqlite3_authorizer(void *autharg, int action, const char *arg1, c
21512193
}
21522194

21532195
int authreturn = SQLITE_DENY;
2196+
bool bailout = false;
21542197

2198+
db_obj->in_callback++;
2199+
zend_try {
21552200
zend_call_known_fcc(&db_obj->authorizer_fcc, &retval, /* argc */ 5, argv, /* named_params */ NULL);
2201+
} zend_catch {
2202+
bailout = true;
2203+
} zend_end_try();
2204+
db_obj->in_callback--;
21562205
if (Z_ISUNDEF(retval)) {
21572206
php_sqlite3_error(db_obj, 0, "An error occurred while invoking the authorizer callback");
21582207
} else {
@@ -2176,6 +2225,9 @@ static int php_sqlite3_authorizer(void *autharg, int action, const char *arg1, c
21762225
zval_ptr_dtor(&argv[3]);
21772226
zval_ptr_dtor(&argv[4]);
21782227

2228+
if (bailout) {
2229+
zend_bailout();
2230+
}
21792231
return authreturn;
21802232
}
21812233
/* }}} */

‎ext/sqlite3/tests/sqlite3_close_from_callback.phpt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
SQLite3::close() from within a UDF callback must not corrupt active statement
3+
--EXTENSIONS--
4+
sqlite3
5+
--FILE--
6+
<?php
7+
$db = new SQLite3(':memory:');
8+
$db->createFunction('boom', function () use ($db) {
9+
try {
10+
var_dump($db->close());
11+
} catch (Throwable $e) {
12+
echo $e::class, ": ", $e->getMessage(), "\n";
13+
}
14+
return 1;
15+
});
16+
$stmt = $db->prepare('SELECT boom()');
17+
var_dump($stmt !== false);
18+
$res = $stmt->execute();
19+
var_dump($res !== false);
20+
var_dump($res->fetchArray(SQLITE3_NUM));
21+
$res->finalize();
22+
var_dump($db->close());
23+
echo "done\n";
24+
?>
25+
--EXPECT--
26+
bool(true)
27+
Error: Cannot close SQLite3 database while inside a callback
28+
bool(true)
29+
Error: Cannot close SQLite3 database while inside a callback
30+
array(1) {
31+
[0]=>
32+
int(1)
33+
}
34+
bool(true)
35+
done

‎ext/sqlite3/tests/sqlite3_close_from_callback_aggregate.phpt‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
SQLite3::close() from within a createAggregate() callback must not corrupt active statement
3+
--EXTENSIONS--
4+
sqlite3
5+
--FILE--
6+
<?php
7+
$db = new SQLite3(':memory:');
8+
$db->exec('CREATE TABLE t (a INTEGER)');
9+
$db->exec('INSERT INTO t VALUES (1)');
10+
$db->exec('INSERT INTO t VALUES (2)');
11+
12+
$reported = false;
13+
$db->createAggregate('agg', function ($context, $rows, $a) use (&$db, &$reported) {
14+
if (!$reported) {
15+
$reported = true;
16+
try {
17+
var_dump($db->close());
18+
} catch (Throwable $e) {
19+
echo $e::class, ": ", $e->getMessage(), "\n";
20+
}
21+
}
22+
return (int) $context + $a;
23+
}, function ($context, $rows) {
24+
return $context;
25+
}, 1);
26+
27+
$stmt = $db->prepare('SELECT agg(a) FROM t');
28+
$res = $stmt->execute();
29+
var_dump($res->fetchArray(SQLITE3_NUM));
30+
$res->finalize();
31+
var_dump($db->close());
32+
echo "done\n";
33+
?>
34+
--EXPECT--
35+
Error: Cannot close SQLite3 database while inside a callback
36+
array(1) {
37+
[0]=>
38+
int(3)
39+
}
40+
bool(true)
41+
done

‎ext/sqlite3/tests/sqlite3_close_from_callback_authorizer.phpt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
SQLite3::close() from within a setAuthorizer() callback must not finalize live statements
3+
--EXTENSIONS--
4+
sqlite3
5+
--FILE--
6+
<?php
7+
$db = new SQLite3(':memory:');
8+
$db->exec('CREATE TABLE t (a INTEGER)');
9+
$db->exec('INSERT INTO t VALUES (7)');
10+
$live = $db->prepare('SELECT a FROM t');
11+
12+
$reported = false;
13+
$db->setAuthorizer(function ($action, $arg1, $arg2, $arg3, $arg4) use (&$db, &$reported) {
14+
if (!$reported) {
15+
$reported = true;
16+
try {
17+
var_dump($db->close());
18+
} catch (Throwable $e) {
19+
echo $e::class, ": ", $e->getMessage(), "\n";
20+
}
21+
}
22+
return SQLite3::OK;
23+
});
24+
25+
$db->prepare('SELECT 1');
26+
27+
$res = $live->execute();
28+
var_dump($res->fetchArray(SQLITE3_NUM));
29+
$res->finalize();
30+
echo "done\n";
31+
?>
32+
--EXPECT--
33+
Error: Cannot close SQLite3 database while inside a callback
34+
array(1) {
35+
[0]=>
36+
int(7)
37+
}
38+
done

‎ext/sqlite3/tests/sqlite3_close_from_callback_collation.phpt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
SQLite3::close() from within a createCollation() callback must not corrupt active statement
3+
--EXTENSIONS--
4+
sqlite3
5+
--FILE--
6+
<?php
7+
$db = new SQLite3(':memory:');
8+
$db->exec('CREATE TABLE t (s TEXT)');
9+
$db->exec("INSERT INTO t VALUES ('b')");
10+
$db->exec("INSERT INTO t VALUES ('a')");
11+
12+
$reported = false;
13+
$db->createCollation('CB', function ($x, $y) use (&$db, &$reported) {
14+
if (!$reported) {
15+
$reported = true;
16+
try {
17+
var_dump($db->close());
18+
} catch (Throwable $e) {
19+
echo $e::class, ": ", $e->getMessage(), "\n";
20+
}
21+
}
22+
return strcmp($x, $y);
23+
});
24+
25+
$stmt = $db->prepare('SELECT s FROM t ORDER BY s COLLATE CB');
26+
$res = $stmt->execute();
27+
while ($row = $res->fetchArray(SQLITE3_NUM)) {
28+
echo $row[0], "\n";
29+
}
30+
$res->finalize();
31+
var_dump($db->close());
32+
echo "done\n";
33+
?>
34+
--EXPECT--
35+
Error: Cannot close SQLite3 database while inside a callback
36+
a
37+
b
38+
bool(true)
39+
done

0 commit comments

Comments
 (0)