Skip to content

Commit 7f321a1

Browse files
GirgiasGirgias
committed
Merge branch 'PHP-8.3' into PHP-8.4
* PHP-8.3: ext/pdo: Fix a UAF when changing default fetch class ctor args
2 parents fc7c353 + 3027600 commit 7f321a1

8 files changed

Lines changed: 423 additions & 1 deletion

‎NEWS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ PHP NEWS
7474
- PDO:
7575
. Fixed a memory leak when the GC is used to free a PDOStatment. (Girgias)
7676
. Fixed a crash in the PDO Firebird Statement destructor. (nielsdos)
77+
. Fixed UAFs when changing default fetch class ctor args. (Girgias, nielsdos)
7778

7879
- PgSql:
7980
. Fixed build failure when the constant PGRES_TUPLES_CHUNK is not present

‎ext/pdo/pdo_stmt.c‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,7 @@ PHP_METHOD(PDOStatement, fetchAll)
12471247
zval *arg2 = NULL;
12481248
zend_class_entry *old_ce;
12491249
zval old_ctor_args, *ctor_args = NULL;
1250+
HashTable *current_ctor = NULL;
12501251
bool error = false;
12511252
int flags, old_arg_count;
12521253

@@ -1264,6 +1265,10 @@ PHP_METHOD(PDOStatement, fetchAll)
12641265

12651266
old_ce = stmt->fetch.cls.ce;
12661267
ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args);
1268+
if (Z_TYPE(old_ctor_args) == IS_ARRAY) {
1269+
/* Protect against destruction by marking this as immutable: we consider this non-owned temporarily */
1270+
Z_TYPE_INFO(stmt->fetch.cls.ctor_args) = IS_ARRAY;
1271+
}
12671272
old_arg_count = stmt->fetch.cls.fci.param_count;
12681273

12691274
do_fetch_opt_finish(stmt, 0);
@@ -1288,7 +1293,13 @@ PHP_METHOD(PDOStatement, fetchAll)
12881293
}
12891294

12901295
if (ctor_args && zend_hash_num_elements(Z_ARRVAL_P(ctor_args)) > 0) {
1291-
ZVAL_COPY_VALUE(&stmt->fetch.cls.ctor_args, ctor_args); /* we're not going to free these */
1296+
/* We increase the refcount and store it in case usercode has been messing around with the ctor args.
1297+
* We need to store current_ctor separately as usercode may change the ctor_args which will cause a leak. */
1298+
current_ctor = Z_ARRVAL_P(ctor_args);
1299+
ZVAL_COPY(&stmt->fetch.cls.ctor_args, ctor_args);
1300+
/* Protect against destruction by marking this as immutable: we consider this non-owned
1301+
* as destruction is handled via current_ctor. */
1302+
Z_TYPE_INFO(stmt->fetch.cls.ctor_args) = IS_ARRAY;
12921303
} else {
12931304
ZVAL_UNDEF(&stmt->fetch.cls.ctor_args);
12941305
}
@@ -1360,6 +1371,7 @@ PHP_METHOD(PDOStatement, fetchAll)
13601371
}
13611372

13621373
PDO_STMT_CLEAR_ERR();
1374+
13631375
if ((how & PDO_FETCH_GROUP) || how == PDO_FETCH_KEY_PAIR ||
13641376
(how == PDO_FETCH_USE_DEFAULT && stmt->default_fetch_type == PDO_FETCH_KEY_PAIR)
13651377
) {
@@ -1384,9 +1396,15 @@ PHP_METHOD(PDOStatement, fetchAll)
13841396
}
13851397

13861398
do_fetch_opt_finish(stmt, 0);
1399+
if (current_ctor) {
1400+
zend_array_release(current_ctor);
1401+
}
13871402

13881403
/* Restore defaults which were changed by PDO_FETCH_CLASS mode */
13891404
stmt->fetch.cls.ce = old_ce;
1405+
/* ctor_args may have been changed to an owned object in the meantime, so destroy it.
1406+
* If it was not, then the type flags update will have protected us against destruction. */
1407+
zval_ptr_dtor(&stmt->fetch.cls.ctor_args);
13901408
ZVAL_COPY_VALUE(&stmt->fetch.cls.ctor_args, &old_ctor_args);
13911409
stmt->fetch.cls.fci.param_count = old_arg_count;
13921410

‎ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch1.phpt‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
--TEST--
2+
PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetch()
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
$db = PDOTest::factory();
17+
18+
class Test {
19+
public string $val1;
20+
public string $val2;
21+
22+
public function __construct(mixed $v) {
23+
var_dump($v);
24+
if ($v instanceof PDOStatement) {
25+
$v->setFetchMode(PDO::FETCH_CLASS, 'Test', [$this->val2]);
26+
}
27+
}
28+
}
29+
30+
$db->exec('CREATE TABLE pdo_fetch_class_change_ctor_one(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
31+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_one VALUES(1, 'A', 'alpha')");
32+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_one VALUES(2, 'B', 'beta')");
33+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_one VALUES(3, 'C', 'gamma')");
34+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_one VALUES(4, 'D', 'delta')");
35+
36+
$stmt = $db->prepare('SELECT val1, val2 FROM pdo_fetch_class_change_ctor_one');
37+
$stmt->setFetchMode(PDO::FETCH_CLASS, 'Test', [$stmt]);
38+
39+
$stmt->execute();
40+
var_dump($stmt->fetch());
41+
42+
?>
43+
--CLEAN--
44+
<?php
45+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
46+
$db = PDOTest::factory();
47+
PDOTest::dropTableIfExists($db, "pdo_fetch_class_change_ctor_one");
48+
?>
49+
--EXPECTF--
50+
object(PDOStatement)#%d (1) {
51+
["queryString"]=>
52+
string(54) "SELECT val1, val2 FROM pdo_fetch_class_change_ctor_one"
53+
}
54+
object(Test)#%d (2) {
55+
["val1"]=>
56+
string(1) "A"
57+
["val2"]=>
58+
string(5) "alpha"
59+
}

‎ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch2.phpt‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
--TEST--
2+
PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetchObject()
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
$db = PDOTest::factory();
17+
18+
class Test {
19+
public string $val1;
20+
public string $val2;
21+
22+
public function __construct(mixed $v) {
23+
var_dump($v);
24+
if ($v instanceof PDOStatement) {
25+
$v->setFetchMode(PDO::FETCH_CLASS, 'Test', [$this->val2]);
26+
}
27+
}
28+
}
29+
30+
// TODO Rename pdo_fetch_class_change_ctor_two table to pdo_fetch_class_change_ctor_two in PHP-8.4
31+
$db->exec('CREATE TABLE pdo_fetch_class_change_ctor_two(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
32+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_two VALUES(1, 'A', 'alpha')");
33+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_two VALUES(2, 'B', 'beta')");
34+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_two VALUES(3, 'C', 'gamma')");
35+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_two VALUES(4, 'D', 'delta')");
36+
37+
$stmt = $db->prepare('SELECT val1, val2 FROM pdo_fetch_class_change_ctor_two');
38+
39+
$stmt->execute();
40+
var_dump($stmt->fetchObject('Test', [$stmt]));
41+
42+
?>
43+
--CLEAN--
44+
<?php
45+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
46+
$db = PDOTest::factory();
47+
PDOTest::dropTableIfExists($db, "pdo_fetch_class_change_ctor_two");
48+
?>
49+
--EXPECTF--
50+
object(PDOStatement)#%s (1) {
51+
["queryString"]=>
52+
string(54) "SELECT val1, val2 FROM pdo_fetch_class_change_ctor_two"
53+
}
54+
object(Test)#%s (2) {
55+
["val1"]=>
56+
string(1) "A"
57+
["val2"]=>
58+
string(5) "alpha"
59+
}

‎ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch3.phpt‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
--TEST--
2+
PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetchAll() (no args variation)
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
$db = PDOTest::factory();
17+
18+
class Test {
19+
public string $val1;
20+
public string $val2;
21+
22+
public function __construct(mixed $v) {
23+
var_dump($v);
24+
if ($v instanceof PDOStatement) {
25+
$v->setFetchMode(PDO::FETCH_CLASS, 'Test', [$this->val2]);
26+
}
27+
}
28+
}
29+
30+
$db->exec('CREATE TABLE pdo_fetch_class_change_ctor_three(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
31+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_three VALUES(1, 'A', 'alpha')");
32+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_three VALUES(2, 'B', 'beta')");
33+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_three VALUES(3, 'C', 'gamma')");
34+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_three VALUES(4, 'D', 'delta')");
35+
36+
$stmt = $db->prepare('SELECT val1, val2 FROM pdo_fetch_class_change_ctor_three');
37+
$stmt->setFetchMode(PDO::FETCH_CLASS, 'Test', [$stmt]);
38+
39+
$stmt->execute();
40+
var_dump($stmt->fetchAll());
41+
42+
?>
43+
--CLEAN--
44+
<?php
45+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
46+
$db = PDOTest::factory();
47+
PDOTest::dropTableIfExists($db, "pdo_fetch_class_change_ctor_three");
48+
?>
49+
--EXPECTF--
50+
object(PDOStatement)#%d (1) {
51+
["queryString"]=>
52+
string(56) "SELECT val1, val2 FROM pdo_fetch_class_change_ctor_three"
53+
}
54+
string(5) "alpha"
55+
string(5) "alpha"
56+
string(5) "alpha"
57+
array(4) {
58+
[0]=>
59+
object(Test)#%d (2) {
60+
["val1"]=>
61+
string(1) "A"
62+
["val2"]=>
63+
string(5) "alpha"
64+
}
65+
[1]=>
66+
object(Test)#%d (2) {
67+
["val1"]=>
68+
string(1) "B"
69+
["val2"]=>
70+
string(4) "beta"
71+
}
72+
[2]=>
73+
object(Test)#%d (2) {
74+
["val1"]=>
75+
string(1) "C"
76+
["val2"]=>
77+
string(5) "gamma"
78+
}
79+
[3]=>
80+
object(Test)#%d (2) {
81+
["val1"]=>
82+
string(1) "D"
83+
["val2"]=>
84+
string(5) "delta"
85+
}
86+
}

‎ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch4.phpt‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
--TEST--
2+
PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetchAll() (args in fetchAll)
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
$db = PDOTest::factory();
17+
18+
class Test {
19+
public string $val1;
20+
public string $val2;
21+
22+
public function __construct(mixed $v) {
23+
var_dump($v);
24+
if ($v instanceof PDOStatement) {
25+
$v->setFetchMode(PDO::FETCH_CLASS, 'Test', [$this->val2]);
26+
}
27+
}
28+
}
29+
30+
$db->exec('CREATE TABLE pdo_fetch_class_change_ctor_four(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
31+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_four VALUES(1, 'A', 'alpha')");
32+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_four VALUES(2, 'B', 'beta')");
33+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_four VALUES(3, 'C', 'gamma')");
34+
$db->exec("INSERT INTO pdo_fetch_class_change_ctor_four VALUES(4, 'D', 'delta')");
35+
36+
$stmt = $db->prepare('SELECT val1, val2 FROM pdo_fetch_class_change_ctor_four');
37+
38+
$stmt->execute();
39+
var_dump($stmt->fetchAll(PDO::FETCH_CLASS, 'Test', [$stmt]));
40+
41+
?>
42+
--CLEAN--
43+
<?php
44+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
45+
$db = PDOTest::factory();
46+
PDOTest::dropTableIfExists($db, "pdo_fetch_class_change_ctor_four");
47+
?>
48+
--EXPECTF--
49+
object(PDOStatement)#%d (1) {
50+
["queryString"]=>
51+
string(55) "SELECT val1, val2 FROM pdo_fetch_class_change_ctor_four"
52+
}
53+
string(5) "alpha"
54+
string(5) "alpha"
55+
string(5) "alpha"
56+
array(4) {
57+
[0]=>
58+
object(Test)#%d (2) {
59+
["val1"]=>
60+
string(1) "A"
61+
["val2"]=>
62+
string(5) "alpha"
63+
}
64+
[1]=>
65+
object(Test)#%d (2) {
66+
["val1"]=>
67+
string(1) "B"
68+
["val2"]=>
69+
string(4) "beta"
70+
}
71+
[2]=>
72+
object(Test)#%d (2) {
73+
["val1"]=>
74+
string(1) "C"
75+
["val2"]=>
76+
string(5) "gamma"
77+
}
78+
[3]=>
79+
object(Test)#%d (2) {
80+
["val1"]=>
81+
string(1) "D"
82+
["val2"]=>
83+
string(5) "delta"
84+
}
85+
}

0 commit comments

Comments
 (0)