Make PDO connection construction single-shot

Constructing a PDO handle over an already constructed one re-ran
connection setup; for a persistent connection the second pass freed the
pemalloc'd dbh with efree() and corrupted the heap. Besides a plain
second __construct(), the same sink was reachable by reentering during a
uri: DSN stream open, by retrying a persistent construct that failed
after swapping in the persistent handle, and by a subclass destructor
reentering while a failed persistent connect() unwinds. Track
construction with an is_constructing flag, set as soon as the handle
exists for both __construct() and connect(), carried onto the persistent
handle and cleared only on success; construction is rejected once the
driver is attached or a construction is in progress. A construct that
fails now leaves the handle unusable instead of allowing a retry.

Author: iliaal

ext/pdo/pdo_dbh.c

@@ -321,6 +321,15 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
 		Z_PARAM_ARRAY_OR_NULL(options)
 	ZEND_PARSE_PARAMETERS_END();
 
+	if (current_object != NULL) {
+		pdo_dbh_t *existing_dbh = php_pdo_dbh_fetch_inner(current_object);
+		if (existing_dbh->driver != NULL || existing_dbh->is_constructing) {
+			zend_throw_error(NULL, "%s object is already constructed", ZSTR_VAL(called_scope->name));
+			RETURN_THROWS();
+		}
+		existing_dbh->is_constructing = 1;
+	}
+
 	/* parse the data source name */
 	colon = strchr(data_source, ':');
 
@@ -374,6 +383,7 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
 
 	if (new_zval_object) {
 		dbh = Z_PDO_DBH_P(new_zval_object);
+		dbh->is_constructing = 1;
 	} else {
 		dbh = php_pdo_dbh_fetch_inner(current_object);
 	}
@@ -427,6 +437,7 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
 
 				pdbh->refcount = 1;
 				pdbh->is_persistent = 1;
+				pdbh->is_constructing = dbh->is_constructing;
 				pdbh->persistent_id = pemalloc(plen + 1, 1);
 				memcpy((char *)pdbh->persistent_id, hashkey, plen+1);
 				pdbh->persistent_id_len = plen;
@@ -493,6 +504,7 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
 		}
 
 		dbh->driver = driver;
+		dbh->is_constructing = 0;
 options:
 		if (options) {
 			zval *attr_value;

ext/pdo/php_pdo_driver.h

@@ -465,9 +465,12 @@ struct _pdo_dbh_t {
 	/* bitmap for pdo_param_event(s) to skip in dispatch_param_event */
 	unsigned skip_param_evt:7;
 
+	/* set while the constructor runs; rejects reentrant (re)construction */
+	unsigned is_constructing:1;
+
 	/* the sum of the number of bits here and the bit fields preceding should
 	 * equal 32 */
-	unsigned _reserved_flags:14;
+	unsigned _reserved_flags:13;
 
 	/* data source string used to open this handle */
 	const char *data_source;

ext/pdo_sqlite/tests/pdo_reconstruct_guard.phpt

@@ -0,0 +1,98 @@
+--TEST--
+PDO::__construct() rejects reconstruction, reentrant construction and unsafe retry
+--EXTENSIONS--
+pdo_sqlite
+--FILE--
+<?php
+$persistent = [PDO::ATTR_PERSISTENT => true];
+
+// Sequential reconstruction of a constructed handle (plain and persistent).
+$p = new PDO('sqlite::memory:');
+try {
+    $p->__construct('sqlite::memory:');
+} catch (\Error $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+
+$q = new PDO('sqlite::memory:', null, null, $persistent);
+try {
+    $q->__construct('sqlite::memory:', null, null, $persistent);
+} catch (\Error $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+
+// Reentrant construction: a uri: DSN opens a stream whose userland wrapper
+// calls __construct() again on the same, still-constructing object.
+class ReentWrapper
+{
+    public $context;
+    private int $pos = 0;
+    private string $data = "sqlite::memory:";
+    public function stream_open($path, $mode, $options, &$opened): bool
+    {
+        global $obj;
+        try {
+            $obj->__construct('sqlite::memory:', null, null, [PDO::ATTR_PERSISTENT => true]);
+        } catch (\Error $e) {
+            echo $e::class, ': ', $e->getMessage(), "\n";
+        }
+        return true;
+    }
+    public function stream_read($count): string
+    {
+        $chunk = substr($this->data, $this->pos, $count);
+        $this->pos += strlen($chunk);
+        return $chunk;
+    }
+    public function stream_eof(): bool
+    {
+        return $this->pos >= strlen($this->data);
+    }
+    public function stream_stat()
+    {
+        return [];
+    }
+}
+stream_wrapper_register('reent', ReentWrapper::class);
+$obj = (new ReflectionClass(PDO::class))->newInstanceWithoutConstructor();
+$obj->__construct('uri:reent://x', null, null, $persistent);
+
+// Retry after a failed persistent construct (which already swapped in the
+// persistent handle before the driver factory failed).
+$r = (new ReflectionClass(PDO::class))->newInstanceWithoutConstructor();
+try {
+    $r->__construct('sqlite:/pdo-reconstruct-nonexistent/x.db', null, null, $persistent);
+} catch (\Throwable $e) {
+}
+try {
+    $r->__construct('sqlite::memory:', null, null, $persistent);
+} catch (\Error $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+
+// Destructor reentry while a failed persistent connect() is unwinding.
+class ReconstructOnDestruct extends Pdo\Sqlite
+{
+    public function __destruct()
+    {
+        try {
+            $this->__construct('sqlite::memory:', null, null, [PDO::ATTR_PERSISTENT => true]);
+        } catch (\Error $e) {
+            echo $e::class, ': ', $e->getMessage(), "\n";
+        }
+    }
+}
+try {
+    ReconstructOnDestruct::connect('sqlite:/pdo-reconstruct-nonexistent/x.db', null, null, $persistent);
+} catch (\Throwable $e) {
+}
+
+echo "done\n";
+?>
+--EXPECT--
+Error: PDO object is already constructed
+Error: PDO object is already constructed
+Error: PDO object is already constructed
+Error: PDO object is already constructed
+Error: ReconstructOnDestruct object is already constructed
+done