Skip to content

Commit d97dae6

Browse files
iliaaliliaal
committed
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
1 parent 5c9a67b commit d97dae6

5 files changed

Lines changed: 261 additions & 24 deletions

File tree

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ PHP NEWS
22
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
33
?? ??? ????, PHP 8.4.25
44

5+
- PDO:
6+
. Fixed PDOStatement::setFetchMode() not applying changes atomically.
7+
(iliaal)
8+
59
- Date:
610
. Fixed leak on double DatePeriod::__construct() call. (ilutov)
711

‎ext/pdo/pdo_stmt.c‎

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1717,27 +1717,24 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a
17171717
uint32_t arg1_arg_num = mode_arg_num + 1;
17181718
uint32_t constructor_arg_num = mode_arg_num + 2;
17191719
uint32_t total_num_args = mode_arg_num + variadic_num_args;
1720+
zend_long fetch_type = mode & ~PDO_FETCH_FLAGS;
1721+
zend_long fetch_column = 0;
1722+
zend_class_entry *fetch_class = NULL;
1723+
zend_array *fetch_ctor_args = NULL;
1724+
zend_object *fetch_into = NULL;
1725+
zval old_ctor_args;
1726+
zval old_into;
17201727

1721-
switch (stmt->default_fetch_type) {
1722-
case PDO_FETCH_INTO:
1723-
if (!Z_ISUNDEF(stmt->fetch.into)) {
1724-
zval_ptr_dtor(&stmt->fetch.into);
1725-
ZVAL_UNDEF(&stmt->fetch.into);
1726-
}
1727-
break;
1728-
default:
1729-
;
1730-
}
1731-
1732-
stmt->default_fetch_type = PDO_FETCH_BOTH;
1728+
ZVAL_UNDEF(&old_ctor_args);
1729+
ZVAL_UNDEF(&old_into);
17331730

17341731
flags = mode & PDO_FETCH_FLAGS;
17351732

17361733
if (!pdo_stmt_verify_mode(stmt, mode, mode_arg_num, false)) {
17371734
return false;
17381735
}
17391736

1740-
switch (mode & ~PDO_FETCH_FLAGS) {
1737+
switch (fetch_type) {
17411738
case PDO_FETCH_USE_DEFAULT:
17421739
case PDO_FETCH_LAZY:
17431740
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
17721769
zend_argument_value_error(arg1_arg_num, "must be greater than or equal to 0");
17731770
return false;
17741771
}
1775-
stmt->fetch.column = Z_LVAL(args[0]);
1772+
fetch_column = Z_LVAL(args[0]);
17761773
break;
17771774

17781775
case PDO_FETCH_CLASS: {
17791776
HashTable *constructor_args = NULL;
1780-
/* Undef constructor arguments */
1781-
ZVAL_UNDEF(&stmt->fetch.cls.ctor_args);
17821777
/* Gets its class name from 1st column */
17831778
if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) {
17841779
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
17881783
zend_string_release(func);
17891784
return false;
17901785
}
1791-
stmt->fetch.cls.ce = NULL;
17921786
} else {
17931787
zend_class_entry *cep;
17941788
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
18271821
constructor_args = Z_ARRVAL(args[1]);
18281822
}
18291823
}
1830-
stmt->fetch.cls.ce = cep;
1824+
fetch_class = cep;
18311825

18321826
/* If constructor arguments are present and not empty */
18331827
if (constructor_args) {
1834-
ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(constructor_args));
1828+
if (!cep->constructor) {
1829+
zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments");
1830+
return false;
1831+
}
1832+
fetch_ctor_args = zend_array_dup(constructor_args);
18351833
}
18361834
}
1837-
1838-
do_fetch_class_prepare(stmt);
18391835
break;
18401836
}
18411837
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
18511847
return false;
18521848
}
18531849

1854-
ZVAL_COPY(&stmt->fetch.into, &args[0]);
1850+
fetch_into = Z_OBJ(args[0]);
18551851
break;
18561852
default:
18571853
zend_argument_value_error(mode_arg_num, "must be one of the PDO::FETCH_* constants");
18581854
return false;
18591855
}
18601856

1857+
if ((stmt->default_fetch_type & ~PDO_FETCH_FLAGS) == PDO_FETCH_INTO) {
1858+
ZVAL_COPY_VALUE(&old_into, &stmt->fetch.into);
1859+
ZVAL_UNDEF(&stmt->fetch.into);
1860+
} else if ((stmt->default_fetch_type & ~PDO_FETCH_FLAGS) == PDO_FETCH_CLASS) {
1861+
do_fetch_opt_finish(stmt, 0);
1862+
ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args);
1863+
ZVAL_UNDEF(&stmt->fetch.cls.ctor_args);
1864+
} else {
1865+
do_fetch_opt_finish(stmt, 1);
1866+
}
1867+
1868+
switch (fetch_type) {
1869+
case PDO_FETCH_COLUMN:
1870+
stmt->fetch.column = fetch_column;
1871+
break;
1872+
case PDO_FETCH_CLASS:
1873+
stmt->fetch.cls.ce = fetch_class;
1874+
if (fetch_ctor_args) {
1875+
ZVAL_ARR(&stmt->fetch.cls.ctor_args, fetch_ctor_args);
1876+
} else {
1877+
ZVAL_UNDEF(&stmt->fetch.cls.ctor_args);
1878+
}
1879+
do_fetch_class_prepare(stmt);
1880+
break;
1881+
case PDO_FETCH_INTO:
1882+
ZVAL_OBJ_COPY(&stmt->fetch.into, fetch_into);
1883+
break;
1884+
default:
1885+
break;
1886+
}
1887+
18611888
stmt->default_fetch_type = mode;
18621889

1890+
if (!Z_ISUNDEF(old_into)) {
1891+
zval_ptr_dtor(&old_into);
1892+
}
1893+
if (!Z_ISUNDEF(old_ctor_args)) {
1894+
zval_ptr_dtor(&old_ctor_args);
1895+
}
1896+
18631897
return true;
18641898
}
18651899

@@ -1875,8 +1909,6 @@ PHP_METHOD(PDOStatement, setFetchMode)
18751909

18761910
PHP_STMT_GET_OBJ;
18771911

1878-
do_fetch_opt_finish(stmt, 1);
1879-
18801912
if (!pdo_stmt_setup_fetch_mode(stmt, fetch_mode, 1, args, num_args)) {
18811913
RETURN_THROWS();
18821914
}

‎ext/pdo/tests/pdo_setfetchmode_atomic.phpt‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
--TEST--
2+
PDO Common: Failed setFetchMode must preserve the prior fetch mode and payload
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if ($dir === false) {
9+
die('skip no driver');
10+
}
11+
require_once $dir . 'pdo_test.inc';
12+
PDOTest::skip();
13+
?>
14+
--FILE--
15+
<?php
16+
if (getenv('REDIR_TEST_DIR') === false) {
17+
putenv('REDIR_TEST_DIR=' . __DIR__ . '/../../pdo/tests/');
18+
}
19+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
20+
$pdo = PDOTest::factory();
21+
22+
class FetchClass
23+
{
24+
public int $i;
25+
26+
public function __construct(public string $marker)
27+
{
28+
}
29+
}
30+
31+
$pdo->exec('CREATE TABLE pdo_setfetchmode_atomic (i INT)');
32+
$pdo->exec('INSERT INTO pdo_setfetchmode_atomic (i) VALUES (1)');
33+
$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_atomic');
34+
$row = new stdClass;
35+
$stmt->setFetchMode(PDO::FETCH_INTO, $row);
36+
try {
37+
$stmt->setFetchMode(PDO::FETCH_CLASS);
38+
} catch (Throwable $e) {
39+
echo "into error: ", $e::class, "\n";
40+
}
41+
$r = $stmt->fetch();
42+
43+
echo "into object preserved: ";
44+
var_dump($r === $row);
45+
echo "into value: ", $row->i, "\n";
46+
47+
$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_atomic');
48+
$stmt->setFetchMode(PDO::FETCH_CLASS, FetchClass::class, ['kept']);
49+
try {
50+
$stmt->setFetchMode(PDO::FETCH_CLASS);
51+
} catch (Throwable $e) {
52+
echo "class error: ", $e::class, "\n";
53+
}
54+
$r = $stmt->fetch();
55+
56+
echo "class preserved: ", $r::class, "\n";
57+
echo "constructor argument: ", $r->marker, "\n";
58+
echo "class value: ", $r->i, "\n";
59+
?>
60+
--CLEAN--
61+
<?php
62+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
63+
$pdo = PDOTest::factory();
64+
PDOTest::dropTableIfExists($pdo, 'pdo_setfetchmode_atomic');
65+
?>
66+
--EXPECT--
67+
into error: ArgumentCountError
68+
into object preserved: bool(true)
69+
into value: 1
70+
class error: ArgumentCountError
71+
class preserved: FetchClass
72+
constructor argument: kept
73+
class value: 1

‎ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
--TEST--
2+
PDO Common: setFetchMode autoload re-entry observes the prior complete mode
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if ($dir === false) {
9+
die('skip no driver');
10+
}
11+
require_once $dir . 'pdo_test.inc';
12+
PDOTest::skip();
13+
?>
14+
--FILE--
15+
<?php
16+
if (getenv('REDIR_TEST_DIR') === false) {
17+
putenv('REDIR_TEST_DIR=' . __DIR__ . '/../../pdo/tests/');
18+
}
19+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
20+
$pdo = PDOTest::factory();
21+
22+
class PriorFetchClass
23+
{
24+
public int $i;
25+
26+
public function __construct(public string $marker)
27+
{
28+
}
29+
}
30+
31+
$pdo->exec('CREATE TABLE pdo_setfetchmode_reentrant_fetch (i INT)');
32+
$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_fetch (i) VALUES (1)');
33+
$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_fetch (i) VALUES (2)');
34+
35+
$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_fetch ORDER BY i');
36+
$into = new stdClass;
37+
$stmt->setFetchMode(PDO::FETCH_INTO, $into);
38+
$autoload = function (string $class) use ($stmt, $into): void {
39+
$row = $stmt->fetch();
40+
echo "into re-entry preserved: ";
41+
var_dump($row === $into);
42+
eval("class $class { public int \$i; }");
43+
};
44+
spl_autoload_register($autoload);
45+
$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredIntoClass');
46+
spl_autoload_unregister($autoload);
47+
$row = $stmt->fetch();
48+
echo "into re-entry value: ", $into->i, "\n";
49+
echo "outer into class: ", $row::class, " ", $row->i, "\n";
50+
51+
$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_fetch ORDER BY i');
52+
$stmt->setFetchMode(PDO::FETCH_CLASS, PriorFetchClass::class, ['kept']);
53+
$autoload = function (string $class) use ($stmt): void {
54+
$row = $stmt->fetch();
55+
echo "class re-entry: ", $row::class, " ", $row->marker, " ", $row->i, "\n";
56+
eval("class $class { public int \$i; }");
57+
};
58+
spl_autoload_register($autoload);
59+
$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredFetchClass');
60+
spl_autoload_unregister($autoload);
61+
$row = $stmt->fetch();
62+
echo "outer class: ", $row::class, " ", $row->i, "\n";
63+
?>
64+
--CLEAN--
65+
<?php
66+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
67+
$pdo = PDOTest::factory();
68+
PDOTest::dropTableIfExists($pdo, 'pdo_setfetchmode_reentrant_fetch');
69+
?>
70+
--EXPECT--
71+
into re-entry preserved: bool(true)
72+
into re-entry value: 1
73+
outer into class: ReenteredIntoClass 2
74+
class re-entry: PriorFetchClass kept 1
75+
outer class: ReenteredFetchClass 2

‎ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
--TEST--
2+
PDO Common: setFetchMode autoload re-entry may change the statement safely
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if ($dir === false) {
9+
die('skip no driver');
10+
}
11+
require_once $dir . 'pdo_test.inc';
12+
PDOTest::skip();
13+
?>
14+
--FILE--
15+
<?php
16+
if (getenv('REDIR_TEST_DIR') === false) {
17+
putenv('REDIR_TEST_DIR=' . __DIR__ . '/../../pdo/tests/');
18+
}
19+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
20+
$pdo = PDOTest::factory();
21+
22+
$pdo->exec('CREATE TABLE pdo_setfetchmode_reentrant_setmode (i INT)');
23+
$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_setmode (i) VALUES (1)');
24+
$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_setmode');
25+
$first = new stdClass;
26+
$second = new stdClass;
27+
$stmt->setFetchMode(PDO::FETCH_INTO, $first);
28+
29+
$autoload = function (string $class) use ($stmt, $second): void {
30+
$stmt->setFetchMode(PDO::FETCH_INTO, $second);
31+
eval("class $class { public int \$i; }");
32+
};
33+
spl_autoload_register($autoload);
34+
$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredSetModeClass');
35+
spl_autoload_unregister($autoload);
36+
37+
$row = $stmt->fetch();
38+
echo $row::class, " ", $row->i, "\n";
39+
echo "first changed: ";
40+
var_dump(isset($first->i));
41+
echo "second changed: ";
42+
var_dump(isset($second->i));
43+
?>
44+
--CLEAN--
45+
<?php
46+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
47+
$pdo = PDOTest::factory();
48+
PDOTest::dropTableIfExists($pdo, 'pdo_setfetchmode_reentrant_setmode');
49+
?>
50+
--EXPECT--
51+
ReenteredSetModeClass 1
52+
first changed: bool(false)
53+
second changed: bool(false)

0 commit comments

Comments
 (0)