diff --git a/ext/sqlite3/sqlite3.c b/ext/sqlite3/sqlite3.c index badcfcc29b0c..e3193dea0ef6 100644 --- a/ext/sqlite3/sqlite3.c +++ b/ext/sqlite3/sqlite3.c @@ -44,6 +44,13 @@ static int php_sqlite3_compare_stmt_zval_free(php_sqlite3_free_list **free_list, RETURN_THROWS(); \ } +#define SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, member, class_name, trampoline_fcc) \ + if (!(db_obj) || !(member)) { \ + zend_release_fcall_info_cache((trampoline_fcc)); \ + zend_throw_error(NULL, "The " #class_name " object has not been correctly initialised or is already closed"); \ + RETURN_THROWS(); \ + } + #define SQLITE3_CHECK_INITIALIZED_STMT(member, class_name) \ if (!(member)) { \ zend_throw_error(NULL, "The " #class_name " object has not been correctly initialised or is already closed"); \ @@ -942,13 +949,16 @@ PHP_METHOD(SQLite3, createFunction) zend_long flags = 0; db_obj = Z_SQLITE3_DB_P(object); - if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf|ll", &sql_func, &sql_func_len, &fci, &fcc, &sql_func_num_args, &flags) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF|ll", &sql_func, &sql_func_len, &fci, &fcc, &sql_func_num_args, &flags) == FAILURE) { + zend_release_fcall_info_cache(&fcc); RETURN_THROWS(); } - SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3) + SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc); if (!sql_func_len) { + /* TODO Add warning/ValueError that name cannot be empty? */ + zend_release_fcall_info_cache(&fcc); RETURN_FALSE; } @@ -956,13 +966,6 @@ PHP_METHOD(SQLite3, createFunction) 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) { func->func_name = estrdup(sql_func); - - if (!ZEND_FCC_INITIALIZED(fcc)) { - zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fcc, NULL); - /* Call trampoline has been cleared by zpp. Refetch it, because we want to deal - * with it outselves. It is important that it is not refetched on every call, - * because calls may occur from different scopes. */ - } zend_fcc_dup(&func->func, &fcc); func->argc = sql_func_num_args; @@ -972,6 +975,7 @@ PHP_METHOD(SQLite3, createFunction) RETURN_TRUE; } efree(func); + zend_release_fcall_info_cache(&fcc); RETURN_FALSE; } @@ -990,13 +994,24 @@ PHP_METHOD(SQLite3, createAggregate) zend_long sql_func_num_args = -1; db_obj = Z_SQLITE3_DB_P(object); - if (zend_parse_parameters(ZEND_NUM_ARGS(), "sff|l", &sql_func, &sql_func_len, &step_fci, &step_fcc, &fini_fci, &fini_fcc, &sql_func_num_args) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "sFF|l", &sql_func, &sql_func_len, &step_fci, &step_fcc, &fini_fci, &fini_fcc, &sql_func_num_args) == FAILURE) { + zend_release_fcall_info_cache(&step_fcc); + zend_release_fcall_info_cache(&fini_fcc); RETURN_THROWS(); } - SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3) + /* Cannot use SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE() as we have 2 FCCs */ + if (!db_obj || !db_obj->initialised) { + zend_release_fcall_info_cache(&step_fcc); + zend_release_fcall_info_cache(&fini_fcc); + zend_throw_error(NULL, "The SQLite3 object has not been correctly initialised or is already closed"); + RETURN_THROWS(); + } if (!sql_func_len) { + /* TODO Add warning/ValueError that name cannot be empty? */ + zend_release_fcall_info_cache(&step_fcc); + zend_release_fcall_info_cache(&fini_fcc); RETURN_FALSE; } @@ -1005,19 +1020,7 @@ PHP_METHOD(SQLite3, createAggregate) 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) { func->func_name = estrdup(sql_func); - if (!ZEND_FCC_INITIALIZED(step_fcc)) { - /* Call trampoline has been cleared by zpp. Refetch it, because we want to deal - * with it outselves. It is important that it is not refetched on every call, - * because calls may occur from different scopes. */ - zend_is_callable_ex(&step_fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &step_fcc, NULL); - } zend_fcc_dup(&func->step, &step_fcc); - if (!ZEND_FCC_INITIALIZED(fini_fcc)) { - /* Call trampoline has been cleared by zpp. Refetch it, because we want to deal - * with it outselves. It is important that it is not refetched on every call, - * because calls may occur from different scopes. */ - zend_is_callable_ex(&fini_fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fini_fcc, NULL); - } zend_fcc_dup(&func->fini, &fini_fcc); func->argc = sql_func_num_args; @@ -1027,6 +1030,8 @@ PHP_METHOD(SQLite3, createAggregate) RETURN_TRUE; } efree(func); + zend_release_fcall_info_cache(&step_fcc); + zend_release_fcall_info_cache(&fini_fcc); RETURN_FALSE; } @@ -1044,13 +1049,15 @@ PHP_METHOD(SQLite3, createCollation) zend_fcall_info_cache fcc; db_obj = Z_SQLITE3_DB_P(object); - if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) { RETURN_THROWS(); } - SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3) + SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc); if (!collation_name_len) { + /* TODO Add warning/ValueError that name cannot be empty? */ + zend_release_fcall_info_cache(&fcc); RETURN_FALSE; } @@ -1058,12 +1065,6 @@ PHP_METHOD(SQLite3, createCollation) if (sqlite3_create_collation(db_obj->db, collation_name, SQLITE_UTF8, collation, php_sqlite3_callback_compare) == SQLITE_OK) { collation->collation_name = estrdup(collation_name); - if (!ZEND_FCC_INITIALIZED(fcc)) { - /* Call trampoline has been cleared by zpp. Refetch it, because we want to deal - * with it outselves. It is important that it is not refetched on every call, - * because calls may occur from different scopes. */ - zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fcc, NULL); - } zend_fcc_dup(&collation->cmp_func, &fcc); collation->next = db_obj->collations; @@ -1072,6 +1073,7 @@ PHP_METHOD(SQLite3, createCollation) RETURN_TRUE; } efree(collation); + zend_release_fcall_info_cache(&fcc); RETURN_FALSE; } @@ -1317,10 +1319,10 @@ PHP_METHOD(SQLite3, setAuthorizer) zend_fcall_info_cache fcc; ZEND_PARSE_PARAMETERS_START(1, 1) - Z_PARAM_FUNC_OR_NULL(fci, fcc) + Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(fci, fcc) ZEND_PARSE_PARAMETERS_END(); - SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3) + SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc); /* Clear previously set callback */ if (ZEND_FCC_INITIALIZED(db_obj->authorizer_fcc)) { @@ -1329,14 +1331,7 @@ PHP_METHOD(SQLite3, setAuthorizer) /* Only enable userland authorizer if argument is not NULL */ if (ZEND_FCI_INITIALIZED(fci)) { - if (!ZEND_FCC_INITIALIZED(fcc)) { - zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fcc, NULL); - /* Call trampoline has been cleared by zpp. Refetch it, because we want to deal - * with it outselves. It is important that it is not refetched on every call, - * because calls may occur from different scopes. */ - } - db_obj->authorizer_fcc = fcc; - zend_fcc_addref(&db_obj->authorizer_fcc); + zend_fcc_dup(&db_obj->authorizer_fcc, &fcc); } RETURN_TRUE; diff --git a/ext/sqlite3/tests/sqlite3_trampoline_create_aggregate_no_leak.phpt b/ext/sqlite3/tests/sqlite3_trampoline_create_aggregate_no_leak.phpt new file mode 100644 index 000000000000..e09ff25ff942 --- /dev/null +++ b/ext/sqlite3/tests/sqlite3_trampoline_create_aggregate_no_leak.phpt @@ -0,0 +1,73 @@ +--TEST-- +SQLite3::createAggregate() use F ZPP for trampoline callback and does not leak +--EXTENSIONS-- +sqlite3 +--FILE-- + 0, 'values' => []]; + } + $context['total'] += (int) $arguments[2]; + $context['values'][] = $context['total']; + return $context; + } +} +$o = new TrampolineTest(); +$step = [$o, 'step']; +$finalize = [$o, 'finalize']; + +var_dump($db->createAggregate('', $step, $finalize, 1)); + +try { + var_dump($db->createAggregate('S', $step, $finalize, new stdClass())); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump($db->createAggregate('S', $step, 'no_func', 1)); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump($db->createAggregate('S', 'no_func', $finalize, 1)); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +echo "Invalid SQLite3 object:\n"; +$rc = new ReflectionClass(SQLite3::class); +$obj = $rc->newInstanceWithoutConstructor(); + +try { + var_dump($obj->createAggregate('S', $step, $finalize, 1)); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +var_dump($db->createAggregate('S', $step, $finalize, 1)); + +echo "Closing database\n"; +var_dump($db->close()); +echo "Done\n"; +?> +--EXPECT-- +bool(false) +TypeError: SQLite3::createAggregate(): Argument #4 ($argCount) must be of type int, stdClass given +TypeError: SQLite3::createAggregate(): Argument #3 ($finalCallback) must be a valid callback, function "no_func" not found or invalid function name +TypeError: SQLite3::createAggregate(): Argument #2 ($stepCallback) must be a valid callback, function "no_func" not found or invalid function name +Invalid SQLite3 object: +Error: The SQLite3 object has not been correctly initialised or is already closed +bool(true) +Closing database +bool(true) +Done diff --git a/ext/sqlite3/tests/sqlite3_trampoline_createcollation_no_leak.phpt b/ext/sqlite3/tests/sqlite3_trampoline_createcollation_no_leak.phpt new file mode 100644 index 000000000000..e4856b6c88b8 --- /dev/null +++ b/ext/sqlite3/tests/sqlite3_trampoline_createcollation_no_leak.phpt @@ -0,0 +1,44 @@ +--TEST-- +SQLite3::createFunction use F ZPP for trampoline callback and does not leak +--EXTENSIONS-- +sqlite3 +--FILE-- +createCollation('', $callback)); +try { + var_dump($db->createCollation('NAT', $callback, new stdClass())); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +echo "Invalid SQLite3 object:\n"; +$rc = new ReflectionClass(SQLite3::class); +$obj = $rc->newInstanceWithoutConstructor(); + +try { + var_dump($obj->createCollation('NAT', $callback)); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +var_dump($db->createCollation('NAT', $callback)); + +?> +--EXPECT-- +bool(false) +ArgumentCountError: SQLite3::createCollation() expects exactly 2 arguments, 3 given +Invalid SQLite3 object: +Error: The SQLite3 object has not been correctly initialised or is already closed +bool(true) diff --git a/ext/sqlite3/tests/sqlite3_trampoline_createfunction_no_leak.phpt b/ext/sqlite3/tests/sqlite3_trampoline_createfunction_no_leak.phpt new file mode 100644 index 000000000000..d386296990d7 --- /dev/null +++ b/ext/sqlite3/tests/sqlite3_trampoline_createfunction_no_leak.phpt @@ -0,0 +1,45 @@ +--TEST-- +SQLite3::createFunction use F ZPP for trampoline callback and does not leak +--EXTENSIONS-- +sqlite3 +--FILE-- +createfunction('', $callback)); + +try { + var_dump($db->createfunction('strtoupper', $callback, new stdClass())); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +echo "Invalid SQLite3 object:\n"; +$rc = new ReflectionClass(SQLite3::class); +$obj = $rc->newInstanceWithoutConstructor(); + +try { + var_dump($obj->createfunction('strtoupper', $callback)); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +var_dump($db->createfunction('strtoupper', $callback)); + +?> +--EXPECT-- +bool(false) +TypeError: SQLite3::createFunction(): Argument #3 ($argCount) must be of type int, stdClass given +Invalid SQLite3 object: +Error: The SQLite3 object has not been correctly initialised or is already closed +bool(true) diff --git a/ext/sqlite3/tests/sqlite3_trampoline_setauthorizer_no_leak.phpt b/ext/sqlite3/tests/sqlite3_trampoline_setauthorizer_no_leak.phpt new file mode 100644 index 000000000000..331f4e1e63c9 --- /dev/null +++ b/ext/sqlite3/tests/sqlite3_trampoline_setauthorizer_no_leak.phpt @@ -0,0 +1,41 @@ +--TEST-- +SQLite3::setAuthorizer use F ZPP for trampoline callback and does not leak +--EXTENSIONS-- +sqlite3 +--FILE-- +enableExceptions(true); + +echo "Invalid SQLite3 object:\n"; +$rc = new ReflectionClass(SQLite3::class); +$obj = $rc->newInstanceWithoutConstructor(); + +try { + var_dump($obj->setAuthorizer($callback)); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$db->setAuthorizer($callback); + +?> +DONE +--EXPECT-- +Invalid SQLite3 object: +Error: The SQLite3 object has not been correctly initialised or is already closed +DONE