From d97dae6d76d40663dafa321f59d0cf2a92f52d5a Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Tue, 4 Aug 2026 22:18:51 -0400 Subject: [PATCH] Make PDOStatement::setFetchMode() updates atomic pdo_stmt_setup_fetch_mode() cleared active fetch state before validating the replacement. Failed validation lost FETCH_INTO objects and FETCH_CLASS constructor arguments; autoload re-entry could also observe a mismatched tagged union. Validate the candidate entirely in local storage. Only after validation succeeds, replace the active payload and mode, and release the old payload from a coherent state. Closes GH-23188 --- NEWS | 4 + ext/pdo/pdo_stmt.c | 80 +++++++++++++------ ext/pdo/tests/pdo_setfetchmode_atomic.phpt | 73 +++++++++++++++++ .../pdo_setfetchmode_reentrant_fetch.phpt | 75 +++++++++++++++++ .../pdo_setfetchmode_reentrant_setmode.phpt | 53 ++++++++++++ 5 files changed, 261 insertions(+), 24 deletions(-) create mode 100644 ext/pdo/tests/pdo_setfetchmode_atomic.phpt create mode 100644 ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt create mode 100644 ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt diff --git a/NEWS b/NEWS index 378643836c80..b8af5d080c7e 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.4.25 +- PDO: + . Fixed PDOStatement::setFetchMode() not applying changes atomically. + (iliaal) + - Date: . Fixed leak on double DatePeriod::__construct() call. (ilutov) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 4e41ea40f08b..6bb2b727c2f9 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1717,19 +1717,16 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a uint32_t arg1_arg_num = mode_arg_num + 1; uint32_t constructor_arg_num = mode_arg_num + 2; uint32_t total_num_args = mode_arg_num + variadic_num_args; + zend_long fetch_type = mode & ~PDO_FETCH_FLAGS; + zend_long fetch_column = 0; + zend_class_entry *fetch_class = NULL; + zend_array *fetch_ctor_args = NULL; + zend_object *fetch_into = NULL; + zval old_ctor_args; + zval old_into; - 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); - } - break; - default: - ; - } - - stmt->default_fetch_type = PDO_FETCH_BOTH; + ZVAL_UNDEF(&old_ctor_args); + ZVAL_UNDEF(&old_into); flags = mode & PDO_FETCH_FLAGS; @@ -1737,7 +1734,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a return false; } - switch (mode & ~PDO_FETCH_FLAGS) { + switch (fetch_type) { case PDO_FETCH_USE_DEFAULT: case PDO_FETCH_LAZY: case PDO_FETCH_ASSOC: @@ -1772,13 +1769,11 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a zend_argument_value_error(arg1_arg_num, "must be greater than or equal to 0"); return false; } - stmt->fetch.column = Z_LVAL(args[0]); + fetch_column = Z_LVAL(args[0]); break; case PDO_FETCH_CLASS: { HashTable *constructor_args = NULL; - /* Undef constructor arguments */ - ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); /* Gets its class name from 1st column */ if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) { if (variadic_num_args != 0) { @@ -1788,7 +1783,6 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a zend_string_release(func); return false; } - stmt->fetch.cls.ce = NULL; } else { zend_class_entry *cep; if (variadic_num_args == 0) { @@ -1827,15 +1821,17 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a constructor_args = Z_ARRVAL(args[1]); } } - stmt->fetch.cls.ce = cep; + fetch_class = cep; /* If constructor arguments are present and not empty */ if (constructor_args) { - ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(constructor_args)); + if (!cep->constructor) { + zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments"); + return false; + } + fetch_ctor_args = zend_array_dup(constructor_args); } } - - do_fetch_class_prepare(stmt); break; } case PDO_FETCH_INTO: @@ -1851,15 +1847,53 @@ 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]); + fetch_into = Z_OBJ(args[0]); break; default: zend_argument_value_error(mode_arg_num, "must be one of the PDO::FETCH_* constants"); return false; } + if ((stmt->default_fetch_type & ~PDO_FETCH_FLAGS) == PDO_FETCH_INTO) { + ZVAL_COPY_VALUE(&old_into, &stmt->fetch.into); + ZVAL_UNDEF(&stmt->fetch.into); + } else if ((stmt->default_fetch_type & ~PDO_FETCH_FLAGS) == PDO_FETCH_CLASS) { + do_fetch_opt_finish(stmt, 0); + ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args); + ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); + } else { + do_fetch_opt_finish(stmt, 1); + } + + switch (fetch_type) { + case PDO_FETCH_COLUMN: + stmt->fetch.column = fetch_column; + break; + case PDO_FETCH_CLASS: + stmt->fetch.cls.ce = fetch_class; + if (fetch_ctor_args) { + ZVAL_ARR(&stmt->fetch.cls.ctor_args, fetch_ctor_args); + } else { + ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); + } + do_fetch_class_prepare(stmt); + break; + case PDO_FETCH_INTO: + ZVAL_OBJ_COPY(&stmt->fetch.into, fetch_into); + break; + default: + break; + } + stmt->default_fetch_type = mode; + if (!Z_ISUNDEF(old_into)) { + zval_ptr_dtor(&old_into); + } + if (!Z_ISUNDEF(old_ctor_args)) { + zval_ptr_dtor(&old_ctor_args); + } + return true; } @@ -1875,8 +1909,6 @@ PHP_METHOD(PDOStatement, setFetchMode) PHP_STMT_GET_OBJ; - do_fetch_opt_finish(stmt, 1); - if (!pdo_stmt_setup_fetch_mode(stmt, fetch_mode, 1, args, num_args)) { RETURN_THROWS(); } diff --git a/ext/pdo/tests/pdo_setfetchmode_atomic.phpt b/ext/pdo/tests/pdo_setfetchmode_atomic.phpt new file mode 100644 index 000000000000..8751c27673a5 --- /dev/null +++ b/ext/pdo/tests/pdo_setfetchmode_atomic.phpt @@ -0,0 +1,73 @@ +--TEST-- +PDO Common: Failed setFetchMode must preserve the prior fetch mode and payload +--EXTENSIONS-- +pdo +--SKIPIF-- + +--FILE-- +exec('CREATE TABLE pdo_setfetchmode_atomic (i INT)'); +$pdo->exec('INSERT INTO pdo_setfetchmode_atomic (i) VALUES (1)'); +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_atomic'); +$row = new stdClass; +$stmt->setFetchMode(PDO::FETCH_INTO, $row); +try { + $stmt->setFetchMode(PDO::FETCH_CLASS); +} catch (Throwable $e) { + echo "into error: ", $e::class, "\n"; +} +$r = $stmt->fetch(); + +echo "into object preserved: "; +var_dump($r === $row); +echo "into value: ", $row->i, "\n"; + +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_atomic'); +$stmt->setFetchMode(PDO::FETCH_CLASS, FetchClass::class, ['kept']); +try { + $stmt->setFetchMode(PDO::FETCH_CLASS); +} catch (Throwable $e) { + echo "class error: ", $e::class, "\n"; +} +$r = $stmt->fetch(); + +echo "class preserved: ", $r::class, "\n"; +echo "constructor argument: ", $r->marker, "\n"; +echo "class value: ", $r->i, "\n"; +?> +--CLEAN-- + +--EXPECT-- +into error: ArgumentCountError +into object preserved: bool(true) +into value: 1 +class error: ArgumentCountError +class preserved: FetchClass +constructor argument: kept +class value: 1 diff --git a/ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt b/ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt new file mode 100644 index 000000000000..bc163366fa8d --- /dev/null +++ b/ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt @@ -0,0 +1,75 @@ +--TEST-- +PDO Common: setFetchMode autoload re-entry observes the prior complete mode +--EXTENSIONS-- +pdo +--SKIPIF-- + +--FILE-- +exec('CREATE TABLE pdo_setfetchmode_reentrant_fetch (i INT)'); +$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_fetch (i) VALUES (1)'); +$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_fetch (i) VALUES (2)'); + +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_fetch ORDER BY i'); +$into = new stdClass; +$stmt->setFetchMode(PDO::FETCH_INTO, $into); +$autoload = function (string $class) use ($stmt, $into): void { + $row = $stmt->fetch(); + echo "into re-entry preserved: "; + var_dump($row === $into); + eval("class $class { public int \$i; }"); +}; +spl_autoload_register($autoload); +$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredIntoClass'); +spl_autoload_unregister($autoload); +$row = $stmt->fetch(); +echo "into re-entry value: ", $into->i, "\n"; +echo "outer into class: ", $row::class, " ", $row->i, "\n"; + +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_fetch ORDER BY i'); +$stmt->setFetchMode(PDO::FETCH_CLASS, PriorFetchClass::class, ['kept']); +$autoload = function (string $class) use ($stmt): void { + $row = $stmt->fetch(); + echo "class re-entry: ", $row::class, " ", $row->marker, " ", $row->i, "\n"; + eval("class $class { public int \$i; }"); +}; +spl_autoload_register($autoload); +$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredFetchClass'); +spl_autoload_unregister($autoload); +$row = $stmt->fetch(); +echo "outer class: ", $row::class, " ", $row->i, "\n"; +?> +--CLEAN-- + +--EXPECT-- +into re-entry preserved: bool(true) +into re-entry value: 1 +outer into class: ReenteredIntoClass 2 +class re-entry: PriorFetchClass kept 1 +outer class: ReenteredFetchClass 2 diff --git a/ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt b/ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt new file mode 100644 index 000000000000..7eb68e10136d --- /dev/null +++ b/ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt @@ -0,0 +1,53 @@ +--TEST-- +PDO Common: setFetchMode autoload re-entry may change the statement safely +--EXTENSIONS-- +pdo +--SKIPIF-- + +--FILE-- +exec('CREATE TABLE pdo_setfetchmode_reentrant_setmode (i INT)'); +$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_setmode (i) VALUES (1)'); +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_setmode'); +$first = new stdClass; +$second = new stdClass; +$stmt->setFetchMode(PDO::FETCH_INTO, $first); + +$autoload = function (string $class) use ($stmt, $second): void { + $stmt->setFetchMode(PDO::FETCH_INTO, $second); + eval("class $class { public int \$i; }"); +}; +spl_autoload_register($autoload); +$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredSetModeClass'); +spl_autoload_unregister($autoload); + +$row = $stmt->fetch(); +echo $row::class, " ", $row->i, "\n"; +echo "first changed: "; +var_dump(isset($first->i)); +echo "second changed: "; +var_dump(isset($second->i)); +?> +--CLEAN-- + +--EXPECT-- +ReenteredSetModeClass 1 +first changed: bool(false) +second changed: bool(false)