diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index fa8c33f863731..64ed35af67e6a 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -35,6 +35,11 @@ PHP 8.5 INTERNALS UPGRADE NOTES - ext/libxml . The refcount APIs now return an `unsigned int` instead of an `int`. +- ext/pdo + . Added `php_pdo_stmt_valid_db_obj_handle()` to check if the database object + is still valid. This is useful when a GC cycle is collected and the + database object can be destroyed prior to destroying the statement. + ======================== 4. OpCode changes ======================== diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 782639be0758e..377615f2fe058 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -65,6 +65,13 @@ void pdo_throw_exception(unsigned int driver_errcode, char *driver_errmsg, pdo_e zend_throw_exception_object(&pdo_exception); } +PDO_API bool php_pdo_stmt_valid_db_obj_handle(const pdo_stmt_t *stmt) +{ + return !Z_ISUNDEF(stmt->database_object_handle) + && IS_OBJ_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE(stmt->database_object_handle)]) + && !(OBJ_FLAGS(Z_OBJ(stmt->database_object_handle)) & IS_OBJ_FREE_CALLED); +} + void pdo_raise_impl_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, pdo_error_type sqlstate, const char *supp) /* {{{ */ { pdo_error_type *pdo_err = &dbh->error_code; diff --git a/ext/pdo/php_pdo_driver.h b/ext/pdo/php_pdo_driver.h index e87950220510a..d46340c93f1c1 100644 --- a/ext/pdo/php_pdo_driver.h +++ b/ext/pdo/php_pdo_driver.h @@ -696,4 +696,11 @@ PDO_API bool pdo_get_long_param(zend_long *lval, zval *value); PDO_API bool pdo_get_bool_param(bool *bval, zval *value); PDO_API void pdo_throw_exception(unsigned int driver_errcode, char *driver_errmsg, pdo_error_type *pdo_error); + +/* When a GC cycle is collected, it's possible that the database object is destroyed prior to destroying + * the statement. In that case, accessing the database object will cause a UAF. + * This function checks if the database object is still valid. + * If it is invalid, the internal driver statement data should have been cleared by the native driver API already. */ +PDO_API bool php_pdo_stmt_valid_db_obj_handle(const pdo_stmt_t *stmt); + #endif /* PHP_PDO_DRIVER_H */ diff --git a/ext/pdo_firebird/firebird_statement.c b/ext/pdo_firebird/firebird_statement.c index 294487958a2b5..60a5e60e11bde 100644 --- a/ext/pdo_firebird/firebird_statement.c +++ b/ext/pdo_firebird/firebird_statement.c @@ -158,15 +158,9 @@ static int pdo_firebird_stmt_dtor(pdo_stmt_t *stmt) /* {{{ */ pdo_firebird_stmt *S = (pdo_firebird_stmt*)stmt->driver_data; int result = 1; - /* TODO: for master, move this check to a separate function shared between pdo drivers. - * pdo_pgsql and pdo_mysql do this exact same thing */ - bool server_obj_usable = !Z_ISUNDEF(stmt->database_object_handle) - && IS_OBJ_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE(stmt->database_object_handle)]) - && !(OBJ_FLAGS(Z_OBJ(stmt->database_object_handle)) & IS_OBJ_FREE_CALLED); - /* release the statement. * Note: if the server object is already gone then the statement was closed already as well. */ - if (server_obj_usable && isc_dsql_free_statement(S->H->isc_status, &S->stmt, DSQL_drop)) { + if (php_pdo_stmt_valid_db_obj_handle(stmt) && isc_dsql_free_statement(S->H->isc_status, &S->stmt, DSQL_drop)) { php_firebird_error_stmt(stmt); result = 0; } diff --git a/ext/pdo_mysql/mysql_statement.c b/ext/pdo_mysql/mysql_statement.c index 9c28be41e9edf..cf01791407e2b 100644 --- a/ext/pdo_mysql/mysql_statement.c +++ b/ext/pdo_mysql/mysql_statement.c @@ -95,9 +95,7 @@ static int pdo_mysql_stmt_dtor(pdo_stmt_t *stmt) /* {{{ */ } #endif - if (!S->done && !Z_ISUNDEF(stmt->database_object_handle) - && IS_OBJ_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE(stmt->database_object_handle)]) - && (!(OBJ_FLAGS(Z_OBJ(stmt->database_object_handle)) & IS_OBJ_FREE_CALLED))) { + if (!S->done && php_pdo_stmt_valid_db_obj_handle(stmt)) { while (mysql_more_results(S->H->server)) { MYSQL_RES *res; if (mysql_next_result(S->H->server) != 0) { diff --git a/ext/pdo_odbc/odbc_stmt.c b/ext/pdo_odbc/odbc_stmt.c index 939adbfb202a6..95840ec7f5391 100644 --- a/ext/pdo_odbc/odbc_stmt.c +++ b/ext/pdo_odbc/odbc_stmt.c @@ -136,11 +136,7 @@ static int odbc_stmt_dtor(pdo_stmt_t *stmt) { pdo_odbc_stmt *S = (pdo_odbc_stmt*)stmt->driver_data; - // TODO: Factor this out; pg/mysql/firebird do the same thing - bool server_obj_usable = !Z_ISUNDEF(stmt->database_object_handle) - && IS_OBJ_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE(stmt->database_object_handle)]) - && !(OBJ_FLAGS(Z_OBJ(stmt->database_object_handle)) & IS_OBJ_FREE_CALLED); - if (S->stmt != SQL_NULL_HANDLE && server_obj_usable) { + if (S->stmt != SQL_NULL_HANDLE && php_pdo_stmt_valid_db_obj_handle(stmt)) { if (stmt->executed) { SQLCloseCursor(S->stmt); } diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index 169fa49af4e14..1d5e188cf5430 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -122,9 +122,7 @@ static void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode) static int pgsql_stmt_dtor(pdo_stmt_t *stmt) { pdo_pgsql_stmt *S = (pdo_pgsql_stmt*)stmt->driver_data; - bool server_obj_usable = !Z_ISUNDEF(stmt->database_object_handle) - && IS_OBJ_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE(stmt->database_object_handle)]) - && !(OBJ_FLAGS(Z_OBJ(stmt->database_object_handle)) & IS_OBJ_FREE_CALLED); + bool server_obj_usable = php_pdo_stmt_valid_db_obj_handle(stmt); pgsql_stmt_finish(S, FIN_DISCARD|(server_obj_usable ? FIN_CLOSE|FIN_ABORT : 0));