Skip to content

Commit

Permalink
Introduce php_pdo_stmt_valid_db_obj_handle() (#17567)
Browse files Browse the repository at this point in the history
  • Loading branch information
nielsdos authored Jan 28, 2025
1 parent ad6cdb5 commit 6f4579a
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 18 deletions.
5 changes: 5 additions & 0 deletions UPGRADING.INTERNALS
Original file line number Diff line number Diff line change
Expand Up @@ -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
========================
Expand Down
7 changes: 7 additions & 0 deletions ext/pdo/pdo_dbh.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions ext/pdo/php_pdo_driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
8 changes: 1 addition & 7 deletions ext/pdo_firebird/firebird_statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 1 addition & 3 deletions ext/pdo_mysql/mysql_statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 1 addition & 5 deletions ext/pdo_odbc/odbc_stmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 1 addition & 3 deletions ext/pdo_pgsql/pgsql_statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down

0 comments on commit 6f4579a

Please sign in to comment.