ext/pdo: Fix a UAF when changing default fetch class ctor args

Co-authored-by: Niels Dossche <7771979+nielsdos@users.noreply.github.com>

NEWS

@@ -47,6 +47,7 @@ PHP                                                                        NEWS
 - 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)
+  . Fixed UAFs when changing default fetch class ctor args. (Girgias, nielsdos)
 
 - Phar:
   . Fixed bug GH-17518 (offset overflow phar extractTo()). (nielsdos)

ext/pdo/pdo_stmt.c

@@ -1252,6 +1252,7 @@ PHP_METHOD(PDOStatement, fetchAll)
 	zval *arg2 = NULL;
 	zend_class_entry *old_ce;
 	zval old_ctor_args, *ctor_args = NULL;
+	HashTable *current_ctor = NULL;
 	bool error = false;
 	int flags, old_arg_count;
 
@@ -1269,6 +1270,10 @@ PHP_METHOD(PDOStatement, fetchAll)
 
 	old_ce = stmt->fetch.cls.ce;
 	ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args);
+	if (Z_TYPE(old_ctor_args) == IS_ARRAY) {
+		/* Protect against destruction by marking this as immutable: we consider this non-owned temporarily */
+		Z_TYPE_INFO(stmt->fetch.cls.ctor_args) = IS_ARRAY;
+	}
 	old_arg_count = stmt->fetch.cls.fci.param_count;
 
 	do_fetch_opt_finish(stmt, 0);
@@ -1293,7 +1298,13 @@ PHP_METHOD(PDOStatement, fetchAll)
 			}
 
 			if (ctor_args && zend_hash_num_elements(Z_ARRVAL_P(ctor_args)) > 0) {
-				ZVAL_COPY_VALUE(&stmt->fetch.cls.ctor_args, ctor_args); /* we're not going to free these */
+				/* We increase the refcount and store it in case usercode has been messing around with the ctor args.
+				 * We need to store current_ctor separately as usercode may change the ctor_args which will cause a leak. */
+				current_ctor = Z_ARRVAL_P(ctor_args);
+				ZVAL_COPY(&stmt->fetch.cls.ctor_args, ctor_args);
+				/* Protect against destruction by marking this as immutable: we consider this non-owned
+				 * as destruction is handled via current_ctor. */
+				Z_TYPE_INFO(stmt->fetch.cls.ctor_args) = IS_ARRAY;
 			} else {
 				ZVAL_UNDEF(&stmt->fetch.cls.ctor_args);
 			}
@@ -1365,6 +1376,7 @@ PHP_METHOD(PDOStatement, fetchAll)
 	}
 
 	PDO_STMT_CLEAR_ERR();
+
 	if ((how & PDO_FETCH_GROUP) || how == PDO_FETCH_KEY_PAIR ||
 		(how == PDO_FETCH_USE_DEFAULT && stmt->default_fetch_type == PDO_FETCH_KEY_PAIR)
 	) {
@@ -1389,9 +1401,15 @@ PHP_METHOD(PDOStatement, fetchAll)
 	}
 
 	do_fetch_opt_finish(stmt, 0);
+	if (current_ctor) {
+		zend_array_release(current_ctor);
+	}
 
 	/* Restore defaults which were changed by PDO_FETCH_CLASS mode */
 	stmt->fetch.cls.ce = old_ce;
+	/* ctor_args may have been changed to an owned object in the meantime, so destroy it.
+	 * If it was not, then the type flags update will have protected us against destruction. */
+	zval_ptr_dtor(&stmt->fetch.cls.ctor_args);
 	ZVAL_COPY_VALUE(&stmt->fetch.cls.ctor_args, &old_ctor_args);
 	stmt->fetch.cls.fci.param_count = old_arg_count;
 

ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch1.phpt

@@ -0,0 +1,54 @@
+--TEST--
+PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetch()
+--EXTENSIONS--
+pdo
+--SKIPIF--
+<?php
+$dir = getenv('REDIR_TEST_DIR');
+if (false == $dir) die('skip no driver');
+require_once $dir . 'pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
+require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+$db = PDOTest::factory();
+
+class Test {
+    public string $val1;
+    public string $val2;
+
+    public function __construct(mixed $v) {
+        var_dump($v);
+        if ($v instanceof PDOStatement) {
+            $v->setFetchMode(PDO::FETCH_CLASS, 'Test', [$this->val2]);
+        }
+    }
+}
+
+// TODO Rename test table to pdo_fetch_class_change_ctor_one in PHP-8.4
+$db->exec('CREATE TABLE test(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
+$db->exec("INSERT INTO test VALUES(1, 'A', 'alpha')");
+$db->exec("INSERT INTO test VALUES(2, 'B', 'beta')");
+$db->exec("INSERT INTO test VALUES(3, 'C', 'gamma')");
+$db->exec("INSERT INTO test VALUES(4, 'D', 'delta')");
+
+$stmt = $db->prepare('SELECT val1, val2 FROM test');
+$stmt->setFetchMode(PDO::FETCH_CLASS, 'Test', [$stmt]);
+
+$stmt->execute();
+var_dump($stmt->fetch());
+
+?>
+--EXPECTF--
+object(PDOStatement)#%d (1) {
+  ["queryString"]=>
+  string(27) "SELECT val1, val2 FROM test"
+}
+object(Test)#%d (2) {
+  ["val1"]=>
+  string(1) "A"
+  ["val2"]=>
+  string(5) "alpha"
+}

ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch2.phpt

@@ -0,0 +1,53 @@
+--TEST--
+PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetchObject()
+--EXTENSIONS--
+pdo
+--SKIPIF--
+<?php
+$dir = getenv('REDIR_TEST_DIR');
+if (false == $dir) die('skip no driver');
+require_once $dir . 'pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
+require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+$db = PDOTest::factory();
+
+class Test {
+    public string $val1;
+    public string $val2;
+
+    public function __construct(mixed $v) {
+        var_dump($v);
+        if ($v instanceof PDOStatement) {
+            $v->setFetchMode(PDO::FETCH_CLASS, 'Test', [$this->val2]);
+        }
+    }
+}
+
+// TODO Rename test table to pdo_fetch_class_change_ctor_two in PHP-8.4
+$db->exec('CREATE TABLE test(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
+$db->exec("INSERT INTO test VALUES(1, 'A', 'alpha')");
+$db->exec("INSERT INTO test VALUES(2, 'B', 'beta')");
+$db->exec("INSERT INTO test VALUES(3, 'C', 'gamma')");
+$db->exec("INSERT INTO test VALUES(4, 'D', 'delta')");
+
+$stmt = $db->prepare('SELECT val1, val2 FROM test');
+
+$stmt->execute();
+var_dump($stmt->fetchObject('Test', [$stmt]));
+
+?>
+--EXPECTF--
+object(PDOStatement)#%s (1) {
+  ["queryString"]=>
+  string(27) "SELECT val1, val2 FROM test"
+}
+object(Test)#%s (2) {
+  ["val1"]=>
+  string(1) "A"
+  ["val2"]=>
+  string(5) "alpha"
+}

ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch3.phpt

@@ -0,0 +1,81 @@
+--TEST--
+PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetchAll() (no args variation)
+--EXTENSIONS--
+pdo
+--SKIPIF--
+<?php
+$dir = getenv('REDIR_TEST_DIR');
+if (false == $dir) die('skip no driver');
+require_once $dir . 'pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
+require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+$db = PDOTest::factory();
+
+class Test {
+    public string $val1;
+    public string $val2;
+
+    public function __construct(mixed $v) {
+        var_dump($v);
+        if ($v instanceof PDOStatement) {
+            $v->setFetchMode(PDO::FETCH_CLASS, 'Test', [$this->val2]);
+        }
+    }
+}
+
+// TODO Rename test table to pdo_fetch_class_change_ctor_three in PHP-8.4
+$db->exec('CREATE TABLE test(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
+$db->exec("INSERT INTO test VALUES(1, 'A', 'alpha')");
+$db->exec("INSERT INTO test VALUES(2, 'B', 'beta')");
+$db->exec("INSERT INTO test VALUES(3, 'C', 'gamma')");
+$db->exec("INSERT INTO test VALUES(4, 'D', 'delta')");
+
+$stmt = $db->prepare('SELECT val1, val2 FROM test');
+$stmt->setFetchMode(PDO::FETCH_CLASS, 'Test', [$stmt]);
+
+$stmt->execute();
+var_dump($stmt->fetchAll());
+
+?>
+--EXPECTF--
+object(PDOStatement)#%d (1) {
+  ["queryString"]=>
+  string(27) "SELECT val1, val2 FROM test"
+}
+string(5) "alpha"
+string(5) "alpha"
+string(5) "alpha"
+array(4) {
+  [0]=>
+  object(Test)#%d (2) {
+    ["val1"]=>
+    string(1) "A"
+    ["val2"]=>
+    string(5) "alpha"
+  }
+  [1]=>
+  object(Test)#%d (2) {
+    ["val1"]=>
+    string(1) "B"
+    ["val2"]=>
+    string(4) "beta"
+  }
+  [2]=>
+  object(Test)#%d (2) {
+    ["val1"]=>
+    string(1) "C"
+    ["val2"]=>
+    string(5) "gamma"
+  }
+  [3]=>
+  object(Test)#%d (2) {
+    ["val1"]=>
+    string(1) "D"
+    ["val2"]=>
+    string(5) "delta"
+  }
+}

ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch4.phpt

@@ -0,0 +1,80 @@
+--TEST--
+PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetchAll() (args in fetchAll)
+--EXTENSIONS--
+pdo
+--SKIPIF--
+<?php
+$dir = getenv('REDIR_TEST_DIR');
+if (false == $dir) die('skip no driver');
+require_once $dir . 'pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
+require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+$db = PDOTest::factory();
+
+class Test {
+    public string $val1;
+    public string $val2;
+
+    public function __construct(mixed $v) {
+        var_dump($v);
+        if ($v instanceof PDOStatement) {
+            $v->setFetchMode(PDO::FETCH_CLASS, 'Test', [$this->val2]);
+        }
+    }
+}
+
+// TODO Rename test table to pdo_fetch_class_change_ctor_four in PHP-8.4
+$db->exec('CREATE TABLE test(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
+$db->exec("INSERT INTO test VALUES(1, 'A', 'alpha')");
+$db->exec("INSERT INTO test VALUES(2, 'B', 'beta')");
+$db->exec("INSERT INTO test VALUES(3, 'C', 'gamma')");
+$db->exec("INSERT INTO test VALUES(4, 'D', 'delta')");
+
+$stmt = $db->prepare('SELECT val1, val2 FROM test');
+
+$stmt->execute();
+var_dump($stmt->fetchAll(PDO::FETCH_CLASS, 'Test', [$stmt]));
+
+?>
+--EXPECTF--
+object(PDOStatement)#%d (1) {
+  ["queryString"]=>
+  string(27) "SELECT val1, val2 FROM test"
+}
+string(5) "alpha"
+string(5) "alpha"
+string(5) "alpha"
+array(4) {
+  [0]=>
+  object(Test)#%d (2) {
+    ["val1"]=>
+    string(1) "A"
+    ["val2"]=>
+    string(5) "alpha"
+  }
+  [1]=>
+  object(Test)#%d (2) {
+    ["val1"]=>
+    string(1) "B"
+    ["val2"]=>
+    string(4) "beta"
+  }
+  [2]=>
+  object(Test)#%d (2) {
+    ["val1"]=>
+    string(1) "C"
+    ["val2"]=>
+    string(5) "gamma"
+  }
+  [3]=>
+  object(Test)#%d (2) {
+    ["val1"]=>
+    string(1) "D"
+    ["val2"]=>
+    string(5) "delta"
+  }
+}

ext/pdo/tests/pdo_fetch_class_change_ctor_args_during_fetch5.phpt

@@ -0,0 +1,50 @@
+--TEST--
+PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetchAll() (via warning and error handler)
+--EXTENSIONS--
+pdo
+--SKIPIF--
+<?php
+$dir = getenv('REDIR_TEST_DIR');
+if (false == $dir) die('skip no driver');
+require_once $dir . 'pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
+require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+$db = PDOTest::factory();
+
+// Warning to hook into error handler
+$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
+
+class B {
+    public function __construct() {}
+}
+
+// TODO Rename test table to pdo_fetch_class_change_ctor_five in PHP-8.4
+$db->exec('CREATE TABLE test(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
+$db->exec("INSERT INTO test VALUES(1, 'A', 'alpha')");
+$db->exec("INSERT INTO test VALUES(2, 'B', 'beta')");
+$db->exec("INSERT INTO test VALUES(3, 'C', 'gamma')");
+$db->exec("INSERT INTO test VALUES(4, 'D', 'delta')");
+
+$stmt = $db->prepare('SELECT val1, val2 FROM test');
+$stmt->execute();
+
+function stuffingErrorHandler(int $errno, string $errstr, string $errfile, int $errline) {
+    global $stmt;
+    $stmt->setFetchMode(PDO::FETCH_CLASS, 'B', [$errstr]);
+    echo $errstr, PHP_EOL;
+}
+set_error_handler(stuffingErrorHandler(...));
+
+var_dump($stmt->fetchAll(PDO::FETCH_CLASS|PDO::FETCH_SERIALIZE, 'B', [$stmt]));
+
+?>
+--EXPECTF--
+PDOStatement::fetchAll(): The PDO::FETCH_SERIALIZE mode is deprecated
+PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: cannot unserialize class
+PDOStatement::fetchAll(): SQLSTATE[HY000]: General error%S
+array(0) {
+}

ext/pdo/tests/pdo_fetch_class_cyclic_ctor.phpt

@@ -0,0 +1,54 @@
+--TEST--
+PDO Common: PDO::FETCH_CLASS with a constructor that changes the ctor args within PDO::fetch()
+--EXTENSIONS--
+pdo
+--SKIPIF--
+<?php
+$dir = getenv('REDIR_TEST_DIR');
+if (false == $dir) die('skip no driver');
+require_once $dir . 'pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
+require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+$db = PDOTest::factory();
+
+class Test {
+    public string $val1;
+    public string $val2;
+
+    public function __construct(mixed $v) {
+        var_dump($v);
+    }
+}
+
+// TODO Rename test table to pdo_fetch_class_cyclic_ctor in PHP-8.4
+$db->exec('CREATE TABLE test(id int NOT NULL PRIMARY KEY, val1 VARCHAR(10), val2 VARCHAR(10))');
+$db->exec("INSERT INTO test VALUES(1, 'A', 'alpha')");
+$db->exec("INSERT INTO test VALUES(2, 'B', 'beta')");
+$db->exec("INSERT INTO test VALUES(3, 'C', 'gamma')");
+$db->exec("INSERT INTO test VALUES(4, 'D', 'delta')");
+
+$args = [];
+$args[] = &$args;
+
+$stmt = $db->prepare('SELECT val1, val2 FROM test');
+$stmt->setFetchMode(PDO::FETCH_CLASS, 'Test', [$args]);
+
+$stmt->execute();
+var_dump($stmt->fetch());
+
+?>
+--EXPECTF--
+array(1) {
+  [0]=>
+  *RECURSION*
+}
+object(Test)#%d (2) {
+  ["val1"]=>
+  string(1) "A"
+  ["val2"]=>
+  string(5) "alpha"
+}