From 3e6f4702ba2a1e58561b62263a63436d59a4a463 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 24 Jan 2025 15:43:22 +0100 Subject: [PATCH 1/4] Fix GHA config yml error --- .github/workflows/nightly.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index c2816d72cf1..4c8ec23b158 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -427,7 +427,7 @@ jobs: -d zend_extension=opcache.so -d opcache.enable_cli=1 - uses: codecov/codecov-action@v4 - if: !cancelled() + if: ${{ !cancelled() }} with: fail_ci_if_error: true token: ${{ secrets.CODECOV_TOKEN }} @@ -500,7 +500,7 @@ jobs: echo opcache.jit_hot_side_exit=1 >> /etc/php.d/opcache.ini php -v - name: Test AMPHP - if: !cancelled() + if: ${{ !cancelled() }} run: | repositories="amp cache dns file http parallel parser pipeline process serialization socket sync websocket-client websocket-server" X=0 @@ -518,7 +518,7 @@ jobs: done exit $X - name: Test Laravel - if: !cancelled() + if: ${{ !cancelled() }} run: | git clone https://github.com/laravel/framework.git --branch=master --depth=1 cd framework @@ -531,7 +531,7 @@ jobs: exit 1 fi - name: Test ReactPHP - if: !cancelled() + if: ${{ !cancelled() }} run: | repositories="async cache child-process datagram dns event-loop promise promise-stream promise-timer stream" X=0 @@ -549,7 +549,7 @@ jobs: done exit $X - name: Test Revolt PHP - if: !cancelled() + if: ${{ !cancelled() }} run: | git clone https://github.com/revoltphp/event-loop.git --depth=1 cd event-loop @@ -560,7 +560,7 @@ jobs: exit 1 fi - name: Test Symfony - if: !cancelled() && !inputs.skip_symfony + if: ${{ !cancelled() && !inputs.skip_symfony }} run: | git clone https://github.com/symfony/symfony.git --depth=1 cd symfony @@ -581,7 +581,7 @@ jobs: done exit $X - name: Test PHPUnit - if: !cancelled() + if: ${{ !cancelled() }} run: | git clone https://github.com/sebastianbergmann/phpunit.git --branch=main --depth=1 cd phpunit @@ -592,7 +592,7 @@ jobs: exit 1 fi - name: 'Symfony Preloading' - if: !cancelled() && !inputs.skip_symfony + if: ${{ !cancelled() && !inputs.skip_symfony }} run: | php /usr/bin/composer create-project symfony/symfony-demo symfony_demo --no-progress --ignore-platform-reqs cd symfony_demo @@ -600,7 +600,7 @@ jobs: sed -i 's/PHP_SAPI/"cli-server"/g' var/cache/dev/App_KernelDevDebugContainer.preload.php php -d opcache.preload=var/cache/dev/App_KernelDevDebugContainer.preload.php public/index.php - name: Test Wordpress - if: !cancelled() && !inputs.skip_wordpress + if: ${{ !cancelled() && !inputs.skip_wordpress }} run: | git clone https://github.com/WordPress/wordpress-develop.git wordpress --depth=1 cd wordpress 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/4] 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 a7d898221f8..d17a40dbc24 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -2079,8 +2079,11 @@ out: 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 00000000000..01df6e0b46d --- /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/4] 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 1fe894cd631..7f4e7aeed87 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/4] Add NEWS entries Closes GH-17539 --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 1084d620deb..c4fd35de806 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)