From 32fb2607a3a8e26f6e79bef418359fa9a172b312 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Fri, 24 Jan 2025 17:32:23 +0000 Subject: [PATCH 1/7] Zend: Make Closure a proper subtype of callable (#15492) --- UPGRADING | 1 + .../{ => callable}/callable_001.phpt | 0 .../{ => callable}/callable_002.phpt | 0 .../{ => callable}/callable_003.phpt | 0 .../callable/callable_variance_closure.phpt | 15 +++++++++++++++ Zend/zend_inheritance.c | 14 ++++++++++++++ 6 files changed, 30 insertions(+) rename Zend/tests/type_declarations/{ => callable}/callable_001.phpt (100%) rename Zend/tests/type_declarations/{ => callable}/callable_002.phpt (100%) rename Zend/tests/type_declarations/{ => callable}/callable_003.phpt (100%) create mode 100644 Zend/tests/type_declarations/callable/callable_variance_closure.phpt diff --git a/UPGRADING b/UPGRADING index c2aef62354064..086fb975d05df 100644 --- a/UPGRADING +++ b/UPGRADING @@ -65,6 +65,7 @@ PHP 8.5 UPGRADE NOTES ======================================== - Core: + . Closure is now a proper subtype of callable . Added support for Closures in constant expressions. RFC: https://wiki.php.net/rfc/closures_in_const_expr diff --git a/Zend/tests/type_declarations/callable_001.phpt b/Zend/tests/type_declarations/callable/callable_001.phpt similarity index 100% rename from Zend/tests/type_declarations/callable_001.phpt rename to Zend/tests/type_declarations/callable/callable_001.phpt diff --git a/Zend/tests/type_declarations/callable_002.phpt b/Zend/tests/type_declarations/callable/callable_002.phpt similarity index 100% rename from Zend/tests/type_declarations/callable_002.phpt rename to Zend/tests/type_declarations/callable/callable_002.phpt diff --git a/Zend/tests/type_declarations/callable_003.phpt b/Zend/tests/type_declarations/callable/callable_003.phpt similarity index 100% rename from Zend/tests/type_declarations/callable_003.phpt rename to Zend/tests/type_declarations/callable/callable_003.phpt diff --git a/Zend/tests/type_declarations/callable/callable_variance_closure.phpt b/Zend/tests/type_declarations/callable/callable_variance_closure.phpt new file mode 100644 index 0000000000000..d3d0a77fd9a7c --- /dev/null +++ b/Zend/tests/type_declarations/callable/callable_variance_closure.phpt @@ -0,0 +1,15 @@ +--TEST-- +Closure should be covariant with callable +--FILE-- + +OK +--EXPECT-- +OK diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 7abc8119d9ea5..d06d8e96195c5 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -23,6 +23,7 @@ #include "zend_execute.h" #include "zend_inheritance.h" #include "zend_interfaces.h" +#include "zend_closures.h" #include "zend_smart_str.h" #include "zend_operators.h" #include "zend_exceptions.h" @@ -490,6 +491,19 @@ static inheritance_status zend_is_class_subtype_of_type( } } + /* If the parent has 'callable' as a return type, then Closure satisfies the co-variant check */ + if (ZEND_TYPE_FULL_MASK(proto_type) & MAY_BE_CALLABLE) { + if (!fe_ce) fe_ce = lookup_class(fe_scope, fe_class_name); + if (!fe_ce) { + have_unresolved = 1; + } else if (fe_ce == zend_ce_closure) { + track_class_dependency(fe_ce, fe_class_name); + return INHERITANCE_SUCCESS; + } else { + return INHERITANCE_ERROR; + } + } + zend_type *single_type; /* Traverse the list of parent types and check if the current child (FE) From 99f8ec33d9713e43c27681c37a7618db10b7c066 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 21 Jan 2025 15:36:47 +0000 Subject: [PATCH 2/7] ext/pdo: Fix memory leak if GC needs to free PDO Statement --- ext/pdo/pdo_stmt.c | 7 +- ext/pdo/tests/pdo_stmt_cyclic_references.phpt | 132 ++++++++++++++++++ 2 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 ext/pdo/tests/pdo_stmt_cyclic_references.phpt diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index a7d898221f881..d17a40dbc242e 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -2079,8 +2079,11 @@ static zend_function *dbstmt_method_get(zend_object **object_pp, zend_string *me static HashTable *dbstmt_get_gc(zend_object *object, zval **gc_data, int *gc_count) { pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object); - *gc_data = &stmt->fetch.into; - *gc_count = 1; + + zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); + zend_get_gc_buffer_add_zval(gc_buffer, &stmt->database_object_handle); + zend_get_gc_buffer_add_zval(gc_buffer, &stmt->fetch.into); + zend_get_gc_buffer_use(gc_buffer, gc_data, gc_count); /** * If there are no dynamic properties and the default property is 1 (that is, there is only one property diff --git a/ext/pdo/tests/pdo_stmt_cyclic_references.phpt b/ext/pdo/tests/pdo_stmt_cyclic_references.phpt new file mode 100644 index 0000000000000..01df6e0b46d60 --- /dev/null +++ b/ext/pdo/tests/pdo_stmt_cyclic_references.phpt @@ -0,0 +1,132 @@ +--TEST-- +PDO Common: Cyclic PDOStatement child class +--EXTENSIONS-- +pdo +--SKIPIF-- + +--FILE-- +exec('CREATE TABLE test(id INT NOT NULL PRIMARY KEY, val VARCHAR(10), val2 VARCHAR(10))'); +$db->exec("INSERT INTO test VALUES(1, 'A', 'AA')"); +$db->exec("INSERT INTO test VALUES(2, 'B', 'BB')"); +$db->exec("INSERT INTO test VALUES(3, 'C', 'CC')"); + +$db->setAttribute(PDO::ATTR_STATEMENT_CLASS, ['CyclicStatement', [new Ref]]); + +echo "Column fetch:\n"; +$stmt = $db->query('SELECT id, val2, val FROM test'); +$stmt->ref->stmt = $stmt; +$stmt->setFetchMode(PDO::FETCH_COLUMN, 2); +foreach($stmt as $obj) { + var_dump($obj); +} + +echo "Class fetch:\n"; +$stmt = $db->query('SELECT id, val2, val FROM test'); +$stmt->ref->stmt = $stmt; +$stmt->setFetchMode(PDO::FETCH_CLASS, 'TestRow', ['Hello world']); +foreach($stmt as $obj) { + var_dump($obj); +} + +echo "Fetch into:\n"; +$stmt = $db->query('SELECT id, val2, val FROM test'); +$stmt->ref->stmt = $stmt; +$stmt->setFetchMode(PDO::FETCH_INTO, new TestRow('I am being fetch into')); +foreach($stmt as $obj) { + var_dump($obj); +} + +?> +--EXPECTF-- +Column fetch: +string(1) "A" +string(1) "B" +string(1) "C" +Class fetch: +object(TestRow)#%d (4) { + ["id"]=> + string(1) "1" + ["val"]=> + string(1) "A" + ["val2"]=> + string(2) "AA" + ["arg"]=> + string(11) "Hello world" +} +object(TestRow)#%d (4) { + ["id"]=> + string(1) "2" + ["val"]=> + string(1) "B" + ["val2"]=> + string(2) "BB" + ["arg"]=> + string(11) "Hello world" +} +object(TestRow)#%d (4) { + ["id"]=> + string(1) "3" + ["val"]=> + string(1) "C" + ["val2"]=> + string(2) "CC" + ["arg"]=> + string(11) "Hello world" +} +Fetch into: +object(TestRow)#4 (4) { + ["id"]=> + string(1) "1" + ["val"]=> + string(1) "A" + ["val2"]=> + string(2) "AA" + ["arg"]=> + string(21) "I am being fetch into" +} +object(TestRow)#4 (4) { + ["id"]=> + string(1) "2" + ["val"]=> + string(1) "B" + ["val2"]=> + string(2) "BB" + ["arg"]=> + string(21) "I am being fetch into" +} +object(TestRow)#4 (4) { + ["id"]=> + string(1) "3" + ["val"]=> + string(1) "C" + ["val2"]=> + string(2) "CC" + ["arg"]=> + string(21) "I am being fetch into" +} From 2ae897fff7af3a794a31a8aeeeeb4f21f6a41393 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 24 Jan 2025 20:19:42 +0100 Subject: [PATCH 3/7] Fix crash in firebird statement dtor If both the driver object and statement end up in the GC buffer and are freed by the GC, then the destruction order is not deterministic and it is possible that the driver object is freed before the statement. In that case, accessing S->H will cause a UAF. As the resources are already released we simply skip the destruction if the driver object is already destroyed. --- ext/pdo_firebird/firebird_statement.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ext/pdo_firebird/firebird_statement.c b/ext/pdo_firebird/firebird_statement.c index 1fe894cd631df..7f4e7aeed87c0 100644 --- a/ext/pdo_firebird/firebird_statement.c +++ b/ext/pdo_firebird/firebird_statement.c @@ -87,8 +87,15 @@ static int firebird_stmt_dtor(pdo_stmt_t *stmt) /* {{{ */ pdo_firebird_stmt *S = (pdo_firebird_stmt*)stmt->driver_data; int result = 1; - /* release the statement */ - if (isc_dsql_free_statement(S->H->isc_status, &S->stmt, DSQL_drop)) { + /* 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)) { RECORD_ERROR(stmt); result = 0; } From e6d917e4c9822c221cac9ed1c3c22aace2387f8a Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Fri, 24 Jan 2025 20:14:23 +0000 Subject: [PATCH 4/7] Add NEWS entries Closes GH-17539 --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 1084d620deb9a..c4fd35de80641 100644 --- a/NEWS +++ b/NEWS @@ -42,6 +42,10 @@ PHP NEWS - Opcache: . Fixed bug GH-17307 (Internal closure causes JIT failure). (nielsdos) +- PDO: + . Fixed a memory leak when the GC is used to free a PDOStatment. (Girgias) + . Fixed a crash in the PDO Firebird Statement destructor. (nielsdos) + - Phar: . Fixed bug GH-17518 (offset overflow phar extractTo()). (nielsdos) From 77130794a37e59165ed51b8b5981b468037f96c2 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 24 Jan 2025 20:07:22 +0100 Subject: [PATCH 5/7] Update test expectation for different Firebird version libfbclient 5.0.1 with server 4.0.1 has a different error message and code. > Read only sql transaction Closes GH-17565. --- ext/pdo_firebird/tests/transaction_access_mode.phpt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/pdo_firebird/tests/transaction_access_mode.phpt b/ext/pdo_firebird/tests/transaction_access_mode.phpt index 8eb3738ff47bf..5202e17129bf4 100644 --- a/ext/pdo_firebird/tests/transaction_access_mode.phpt +++ b/ext/pdo_firebird/tests/transaction_access_mode.phpt @@ -135,7 +135,7 @@ $dbh = getDbConnection(); @$dbh->exec('DROP TABLE transaction_access_mode'); unset($dbh); ?> ---EXPECT-- +--EXPECTF-- ========== Set attr in construct ========== OK: writable OK: readonly @@ -157,7 +157,7 @@ array(1) { readonly bool(true) OK: readonly -SQLSTATE[42000]: Syntax error or access violation: -817 attempted update during read-only transaction +SQLSTATE[%r(42000|25006)%r]: %r(Read only sql transaction|Syntax error or access violation)%r: -817 attempted update during read-only transaction array(1) { [0]=> array(2) { From 27187bd1da0c3733241472b4ce6d6710ea2610d6 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 24 Jan 2025 23:31:11 +0100 Subject: [PATCH 6/7] [ci skip] NEWS for GH-17122 --- NEWS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS b/NEWS index 5c5e21e8e50ac..d47d03b8e3940 100644 --- a/NEWS +++ b/NEWS @@ -65,6 +65,9 @@ PHP NEWS the cpu mask argument with entries type different than int/string. (David Carlier) +- PCRE: + . Fixed bug GH-17122 (memory leak in regex). (nielsdos) + - PDO: . Fixed a memory leak when the GC is used to free a PDOStatment. (Girgias) . Fixed a crash in the PDO Firebird Statement destructor. (nielsdos) From 6fc49ab518185f57b16ade93242493dd7b15b84f Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Fri, 24 Jan 2025 22:59:56 +0000 Subject: [PATCH 7/7] ext/pdo: Convert FETCH_INTO zval to a zend_object pointer (#17525) --- ext/pdo/pdo_stmt.c | 27 ++++++++++++++++----------- ext/pdo/php_pdo_driver.h | 2 +- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index bc71d3a7205e0..154d8f0a4e3aa 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -836,14 +836,14 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h case PDO_FETCH_INTO: /* TODO: Make this an assertion and ensure this is true higher up? */ - if (Z_ISUNDEF(stmt->fetch.into)) { + if (stmt->fetch.into == NULL) { /* TODO ArgumentCountError? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch-into object specified."); return 0; break; } - ZVAL_COPY(return_value, &stmt->fetch.into); + ZVAL_OBJ_COPY(return_value, stmt->fetch.into); if (Z_OBJ_P(return_value)->ce == ZEND_STANDARD_CLASS_DEF_PTR) { how = PDO_FETCH_OBJ; @@ -1655,9 +1655,9 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a switch (stmt->default_fetch_type) { case PDO_FETCH_INTO: - if (!Z_ISUNDEF(stmt->fetch.into)) { - zval_ptr_dtor(&stmt->fetch.into); - ZVAL_UNDEF(&stmt->fetch.into); + if (stmt->fetch.into) { + OBJ_RELEASE(stmt->fetch.into); + stmt->fetch.into = NULL; } break; default: @@ -1786,7 +1786,8 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a return false; } - ZVAL_COPY(&stmt->fetch.into, &args[0]); + GC_ADDREF(Z_OBJ(args[0])); + stmt->fetch.into = Z_OBJ(args[0]); break; default: zend_argument_value_error(mode_arg_num, "must be one of the PDO::FETCH_* constants"); @@ -2027,10 +2028,15 @@ static zend_function *dbstmt_method_get(zend_object **object_pp, zend_string *me static HashTable *dbstmt_get_gc(zend_object *object, zval **gc_data, int *gc_count) { pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object); + enum pdo_fetch_type default_fetch_mode = stmt->default_fetch_type & ~PDO_FETCH_FLAGS; zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); zend_get_gc_buffer_add_zval(gc_buffer, &stmt->database_object_handle); - zend_get_gc_buffer_add_zval(gc_buffer, &stmt->fetch.into); + if (default_fetch_mode == PDO_FETCH_INTO) { + zend_get_gc_buffer_add_obj(gc_buffer, stmt->fetch.into); + } else if (default_fetch_mode == PDO_FETCH_CLASS) { + zend_get_gc_buffer_add_zval(gc_buffer, &stmt->fetch.cls.ctor_args); + } zend_get_gc_buffer_use(gc_buffer, gc_data, gc_count); /** @@ -2077,9 +2083,9 @@ PDO_API void php_pdo_free_statement(pdo_stmt_t *stmt) pdo_stmt_reset_columns(stmt); - if (!Z_ISUNDEF(stmt->fetch.into) && stmt->default_fetch_type == PDO_FETCH_INTO) { - zval_ptr_dtor(&stmt->fetch.into); - ZVAL_UNDEF(&stmt->fetch.into); + if (stmt->fetch.into && stmt->default_fetch_type == PDO_FETCH_INTO) { + OBJ_RELEASE(stmt->fetch.into); + stmt->fetch.into = NULL; } do_fetch_opt_finish(stmt, 1); @@ -2168,7 +2174,6 @@ static void pdo_stmt_iter_move_forwards(zend_object_iterator *iter) if (!do_fetch(stmt, &I->fetch_ahead, PDO_FETCH_USE_DEFAULT, PDO_FETCH_ORI_NEXT, /* offset */ 0, NULL)) { - PDO_HANDLE_STMT_ERR(); I->key = (zend_ulong)-1; ZVAL_UNDEF(&I->fetch_ahead); diff --git a/ext/pdo/php_pdo_driver.h b/ext/pdo/php_pdo_driver.h index 2d7f4c85034e8..e87950220510a 100644 --- a/ext/pdo/php_pdo_driver.h +++ b/ext/pdo/php_pdo_driver.h @@ -621,7 +621,7 @@ struct _pdo_stmt_t { zval dummy; /* This exists due to alignment reasons with fetch.into and fetch.cls.ctor_args */ zend_fcall_info_cache fcc; } func; - zval into; + zend_object *into; } fetch; /* used by the query parser for driver specific