Skip to content

Commit 357fcf1

Browse files
iliaaliliaal
committed
ext/pdo: Make PDOStatement::setFetchMode() updates atomic
pdo_stmt_setup_fetch_mode() cleared the active fetch state before validating the replacement, so a failed call lost FETCH_INTO objects and FETCH_CLASS constructor arguments, and autoload re-entry could observe a mismatched tagged union. Validate the candidate in local storage and replace the active payload only after validation succeeds. Closes GH-23188
1 parent 71e56ed commit 357fcf1

5 files changed

Lines changed: 262 additions & 27 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ PHP NEWS
137137
options value. (Ilia Alshanetsky)
138138
. Fixed PDOStatement not reporting its bound parameters and columns to the
139139
cycle collector. (Ilia Alshanetsky)
140+
. Fixed PDOStatement::setFetchMode() not applying changes atomically.
141+
(Ilia Alshanetsky)
140142

141143
- PDO_DBLIB:
142144
. Fixed bug GH-23741 (segfault after a failed query inside a PDO

‎ext/pdo/pdo_stmt.c‎

Lines changed: 59 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1772,29 +1772,24 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a
17721772
uint32_t arg1_arg_num = mode_arg_num + 1;
17731773
uint32_t constructor_arg_num = mode_arg_num + 2;
17741774
uint32_t total_num_args = mode_arg_num + variadic_num_args;
1775+
zend_long fetch_type = mode & ~PDO_FETCH_FLAGS;
1776+
zend_long fetch_column = 0;
1777+
zend_class_entry *fetch_class = NULL;
1778+
zend_array *fetch_ctor_args = NULL;
1779+
zend_object *fetch_into = NULL;
1780+
zval old_ctor_args;
1781+
zval old_into;
17751782

1776-
switch (stmt->default_fetch_type) {
1777-
case PDO_FETCH_INTO:
1778-
if (!Z_ISUNDEF(stmt->fetch.into)) {
1779-
zval_ptr_dtor(&stmt->fetch.into);
1780-
ZVAL_UNDEF(&stmt->fetch.into);
1781-
}
1782-
break;
1783-
default:
1784-
;
1785-
}
1786-
1787-
stmt->default_fetch_type = stmt->dbh->default_fetch_type;
1783+
ZVAL_UNDEF(&old_ctor_args);
1784+
ZVAL_UNDEF(&old_into);
17881785

17891786
flags = mode & PDO_FETCH_FLAGS;
17901787

17911788
if (!pdo_stmt_verify_mode(stmt, mode, mode_arg_num, false)) {
17921789
return false;
17931790
}
17941791

1795-
bool use_default = (mode & ~PDO_FETCH_FLAGS) == PDO_FETCH_USE_DEFAULT;
1796-
1797-
switch (mode & ~PDO_FETCH_FLAGS) {
1792+
switch (fetch_type) {
17981793
case PDO_FETCH_USE_DEFAULT:
17991794
case PDO_FETCH_LAZY:
18001795
case PDO_FETCH_ASSOC:
@@ -1829,13 +1824,11 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a
18291824
zend_argument_value_error(arg1_arg_num, "must be greater than or equal to 0");
18301825
return false;
18311826
}
1832-
stmt->fetch.column = Z_LVAL(args[0]);
1827+
fetch_column = Z_LVAL(args[0]);
18331828
break;
18341829

18351830
case PDO_FETCH_CLASS: {
18361831
HashTable *constructor_args = NULL;
1837-
/* Undef constructor arguments */
1838-
ZVAL_UNDEF(&stmt->fetch.cls.ctor_args);
18391832
/* Gets its class name from 1st column */
18401833
if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) {
18411834
if (variadic_num_args != 0) {
@@ -1845,7 +1838,6 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a
18451838
zend_string_release(func);
18461839
return false;
18471840
}
1848-
stmt->fetch.cls.ce = NULL;
18491841
} else {
18501842
zend_class_entry *cep;
18511843
if (variadic_num_args == 0) {
@@ -1884,15 +1876,17 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a
18841876
constructor_args = Z_ARRVAL(args[1]);
18851877
}
18861878
}
1887-
stmt->fetch.cls.ce = cep;
1879+
fetch_class = cep;
18881880

18891881
/* If constructor arguments are present and not empty */
18901882
if (constructor_args) {
1891-
ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(constructor_args));
1883+
if (!cep->constructor) {
1884+
zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments");
1885+
return false;
1886+
}
1887+
fetch_ctor_args = zend_array_dup(constructor_args);
18921888
}
18931889
}
1894-
1895-
do_fetch_class_prepare(stmt);
18961890
break;
18971891
}
18981892
case PDO_FETCH_INTO:
@@ -1908,17 +1902,57 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a
19081902
return false;
19091903
}
19101904

1911-
ZVAL_COPY(&stmt->fetch.into, &args[0]);
1905+
fetch_into = Z_OBJ(args[0]);
19121906
break;
19131907
default:
19141908
zend_argument_value_error(mode_arg_num, "must be one of the PDO::FETCH_* constants");
19151909
return false;
19161910
}
19171911

1918-
if (!use_default) {
1912+
if ((stmt->default_fetch_type & ~PDO_FETCH_FLAGS) == PDO_FETCH_INTO) {
1913+
ZVAL_COPY_VALUE(&old_into, &stmt->fetch.into);
1914+
ZVAL_UNDEF(&stmt->fetch.into);
1915+
} else if ((stmt->default_fetch_type & ~PDO_FETCH_FLAGS) == PDO_FETCH_CLASS) {
1916+
do_fetch_opt_finish(stmt, 0);
1917+
ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args);
1918+
ZVAL_UNDEF(&stmt->fetch.cls.ctor_args);
1919+
} else {
1920+
do_fetch_opt_finish(stmt, 1);
1921+
}
1922+
1923+
switch (fetch_type) {
1924+
case PDO_FETCH_COLUMN:
1925+
stmt->fetch.column = fetch_column;
1926+
break;
1927+
case PDO_FETCH_CLASS:
1928+
stmt->fetch.cls.ce = fetch_class;
1929+
if (fetch_ctor_args) {
1930+
ZVAL_ARR(&stmt->fetch.cls.ctor_args, fetch_ctor_args);
1931+
} else {
1932+
ZVAL_UNDEF(&stmt->fetch.cls.ctor_args);
1933+
}
1934+
do_fetch_class_prepare(stmt);
1935+
break;
1936+
case PDO_FETCH_INTO:
1937+
ZVAL_OBJ_COPY(&stmt->fetch.into, fetch_into);
1938+
break;
1939+
default:
1940+
break;
1941+
}
1942+
1943+
if (fetch_type == PDO_FETCH_USE_DEFAULT) {
1944+
stmt->default_fetch_type = stmt->dbh->default_fetch_type;
1945+
} else {
19191946
stmt->default_fetch_type = mode;
19201947
}
19211948

1949+
if (!Z_ISUNDEF(old_into)) {
1950+
zval_ptr_dtor(&old_into);
1951+
}
1952+
if (!Z_ISUNDEF(old_ctor_args)) {
1953+
zval_ptr_dtor(&old_ctor_args);
1954+
}
1955+
19221956
return true;
19231957
}
19241958

@@ -1934,8 +1968,6 @@ PHP_METHOD(PDOStatement, setFetchMode)
19341968

19351969
PHP_STMT_GET_OBJ;
19361970

1937-
do_fetch_opt_finish(stmt, 1);
1938-
19391971
if (!pdo_stmt_setup_fetch_mode(stmt, fetch_mode, 1, args, num_args)) {
19401972
RETURN_THROWS();
19411973
}

‎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)