[php-src] PHP-8.4: Fix pdo_odbc output-buffer leak and stale-binding out-of-bounds read

Ilia Alshanetsky <[email protected]>
Newsgroups gmane.comp.php.cvs.general
Message-ID <[email protected]>
Author: Ilia Alshanetsky (iliaal)
Date: 2026-07-15T16:11:19-04:00

Commit: https://github.com/php/php-src/commit/bdcf9ceee383a9d2f4624b90b463279e3a7b1368
Raw diff: https://github.com/php/php-src/commit/bdcf9ceee383a9d2f4624b90b463279e3a7b1368.diff

Fix pdo_odbc output-buffer leak and stale-binding out-of-bounds read

SQLBindParameter recorded deferred pointers (P->outbuf, &P->len, and the
bound-parameter struct) into the ODBC statement at PDO_PARAM_EVT_ALLOC, and
P->outbuf was never freed. Both outlive the bound-parameter struct, which
PDO destroys mid-statement on execute() with an array or a rebind: the
buffer leaked and the stale binding made the next SQLExecute read freed
memory. Track output buffers on the statement and free them in the
destructor, and defer binding to execute time, resetting and rebinding the
current parameters when the set changed.

Closes GH-22735

Changed paths:
  A  ext/pdo_odbc/tests/bound_param_rebind.phpt
  A  ext/pdo_odbc/tests/output_param_leak.phpt
  M  ext/pdo_odbc/odbc_stmt.c
  M  ext/pdo_odbc/php_pdo_odbc_int.h


Diff:

diff --git a/ext/pdo_odbc/odbc_stmt.c b/ext/pdo_odbc/odbc_stmt.c
index 8940143da09c..8786f2563e52 100644
--- a/ext/pdo_odbc/odbc_stmt.c
+++ b/ext/pdo_odbc/odbc_stmt.c
@@ -133,6 +133,11 @@ static void free_cols(pdo_stmt_t *stmt, pdo_odbc_stmt *S)
 	}
 }
 
+static void odbc_free_out_buffer(zval *el)
+{
+	efree(Z_PTR_P(el));
+}
+
 static int odbc_stmt_dtor(pdo_stmt_t *stmt)
 {
 	pdo_odbc_stmt *S = (pdo_odbc_stmt*)stmt->driver_data;
@@ -153,11 +158,41 @@ static int odbc_stmt_dtor(pdo_stmt_t *stmt)
 	if (S->convbuf) {
 		efree(S->convbuf);
 	}
+	if (S->out_buffers) {
+		zend_hash_destroy(S->out_buffers);
+		FREE_HASHTABLE(S->out_buffers);
+	}
 	efree(S);
 
 	return 1;
 }
 
+static bool odbc_bind_param(pdo_stmt_t *stmt, struct pdo_bound_param_data *param)
+{
+	pdo_odbc_stmt *S = (pdo_odbc_stmt*)stmt->driver_data;
+	pdo_odbc_param *P = (pdo_odbc_param*)param->driver_data;
+	RETCODE rc;
+
+	if (!P) {
+		return true;
+	}
+
+	rc = SQLBindParameter(S->stmt, (SQLUSMALLINT) param->paramno+1,
+			P->paramtype, P->ctype, P->sqltype, P->precision, P->scale,
+			P->paramtype == SQL_PARAM_INPUT ?
+				(SQLPOINTER)param :
+				P->outbuf,
+			P->outbuf ? P->outbuflen : 0,
+			&P->len
+			);
+
+	if (rc == SQL_SUCCESS || rc == SQL_SUCCESS_WITH_INFO) {
+		return true;
+	}
+	pdo_odbc_stmt_error("SQLBindParameter");
+	return false;
+}
+
 static int odbc_stmt_execute(pdo_stmt_t *stmt)
 {
 	RETCODE rc, rc1;
@@ -169,6 +204,24 @@ static int odbc_stmt_execute(pdo_stmt_t *stmt)
 		SQLCloseCursor(S->stmt);
 	}
 
+	if (S->params_dirty) {
+		rc = SQLFreeStmt(S->stmt, SQL_RESET_PARAMS);
+		if (rc != SQL_SUCCESS && rc != SQL_SUCCESS_WITH_INFO) {
+			pdo_odbc_stmt_error("SQLFreeStmt: SQL_RESET_PARAMS");
+			return 0;
+		}
+		if (stmt->bound_params) {
+			struct pdo_bound_param_data *param;
+
+			ZEND_HASH_FOREACH_PTR(stmt->bound_params, param) {
+				if (!odbc_bind_param(stmt, param)) {
+					return 0;
+				}
+			} ZEND_HASH_FOREACH_END();
+		}
+		S->params_dirty = false;
+	}
+
 	rc = SQLExecute(S->stmt);
 
 	while (rc == SQL_NEED_DATA) {
@@ -312,6 +365,7 @@ static int odbc_stmt_param_hook(pdo_stmt_t *stmt, struct pdo_bound_param_data *p
 				if (P) {
 					efree(P);
 				}
+				S->params_dirty = true;
 				break;
 
 			case PDO_PARAM_EVT_ALLOC:
@@ -386,6 +440,11 @@ static int odbc_stmt_param_hook(pdo_stmt_t *stmt, struct pdo_bound_param_data *p
 						}
 						P->outbuf = emalloc(P->len + (P->is_unicode ? 2:1));
 						P->outbuflen = P->len;
+						if (!S->out_buffers) {
+							ALLOC_HASHTABLE(S->out_buffers);
+							zend_hash_init(S->out_buffers, HT_MIN_SIZE, NULL, odbc_free_out_buffer, false);
+						}
+						zend_hash_next_index_insert_ptr(S->out_buffers, P->outbuf);
 					}
 				}
 
@@ -394,20 +453,12 @@ static int odbc_stmt_param_hook(pdo_stmt_t *stmt, struct pdo_bound_param_data *p
 					return 0;
 				}
 
-				rc = SQLBindParameter(S->stmt, (SQLUSMALLINT) param->paramno+1,
-						P->paramtype, ctype, sqltype, precision, scale,
-						P->paramtype == SQL_PARAM_INPUT ?
-							(SQLPOINTER)param :
-							P->outbuf,
-						P->len,
-						&P->len
-						);
-
-				if (rc == SQL_SUCCESS || rc == SQL_SUCCESS_WITH_INFO) {
-					return 1;
-				}
-				pdo_odbc_stmt_error("SQLBindParameter");
-				return 0;
+				P->sqltype = sqltype;
+				P->ctype = ctype;
+				P->precision = precision;
+				P->scale = scale;
+				S->params_dirty = true;
+				return 1;
 			}
 
 			case PDO_PARAM_EVT_EXEC_PRE:
diff --git a/ext/pdo_odbc/php_pdo_odbc_int.h b/ext/pdo_odbc/php_pdo_odbc_int.h
index ea93c58b2b60..c25fa5e36eae 100644
--- a/ext/pdo_odbc/php_pdo_odbc_int.h
+++ b/ext/pdo_odbc/php_pdo_odbc_int.h
@@ -145,16 +145,22 @@ typedef struct {
 	pdo_odbc_errinfo einfo;
 	char *convbuf;
 	zend_ulong convbufsize;
+	HashTable *out_buffers;
 	unsigned going_long:1;
 	unsigned assume_utf8:1;
+	unsigned params_dirty:1;
 	signed col_count:16;
-	unsigned _spare:14;
+	unsigned _spare:13;
 } pdo_odbc_stmt;
 
 typedef struct {
 	SQLLEN len;
 	SQLLEN outbuflen;
 	SQLSMALLINT paramtype;
+	SQLSMALLINT sqltype;
+	SQLSMALLINT ctype;
+	SQLSMALLINT scale;
+	SQLULEN precision;
 	char *outbuf;
 	unsigned is_unicode:1;
 	unsigned _spare:31;
diff --git a/ext/pdo_odbc/tests/bound_param_rebind.phpt b/ext/pdo_odbc/tests/bound_param_rebind.phpt
new file mode 100644
index 000000000000..1ae9651f3a7d
--- /dev/null
+++ b/ext/pdo_odbc/tests/bound_param_rebind.phpt
@@ -0,0 +1,28 @@
+--TEST--
+pdo_odbc: a bound parameter does not leave a stale ODBC binding on re-execute
+--EXTENSIONS--
+pdo_odbc
+--SKIPIF--
+<?php
+require __DIR__ . '/config.inc';
+try {
+    $pdo = new PDO(PDO_ODBC_SQLITE_DSN);
+} catch (PDOException $e) {
+    die("skip requires the SQLite3 ODBC driver");
+}
+?>
+--FILE--
+<?php
+require __DIR__ . '/config.inc';
+$pdo = new PDO(PDO_ODBC_SQLITE_DSN);
+$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT);
+
+$stmt = $pdo->prepare('SELECT ? AS v');
+$v = 'bound';
+$stmt->bindParam(1, $v, PDO::PARAM_STR | PDO::PARAM_INPUT_OUTPUT, 256);
+$stmt->execute(['from_array']);
+
+var_dump($stmt->fetch(PDO::FETCH_ASSOC)['v']);
+?>
+--EXPECT--
+string(10) "from_array"
diff --git a/ext/pdo_odbc/tests/output_param_leak.phpt b/ext/pdo_odbc/tests/output_param_leak.phpt
new file mode 100644
index 000000000000..f6d4aad5a8c6
--- /dev/null
+++ b/ext/pdo_odbc/tests/output_param_leak.phpt
@@ -0,0 +1,29 @@
+--TEST--
+pdo_odbc: bound output parameter buffer is released (no leak)
+--EXTENSIONS--
+pdo_odbc
+--SKIPIF--
+<?php
+require __DIR__ . '/config.inc';
+try {
+    $pdo = new PDO(PDO_ODBC_SQLITE_DSN);
+} catch (PDOException $e) {
+    die("skip requires the SQLite3 ODBC driver");
+}
+?>
+--FILE--
+<?php
+require __DIR__ . '/config.inc';
+$pdo = new PDO(PDO_ODBC_SQLITE_DSN);
+$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT);
+
+$stmt = $pdo->prepare('SELECT ? AS v');
+$var = 'seed';
+$stmt->bindParam(1, $var, PDO::PARAM_STR | PDO::PARAM_INPUT_OUTPUT, 256);
+$stmt->execute();
+$row = $stmt->fetch(PDO::FETCH_ASSOC);
+
+var_dump($row['v']);
+?>
+--EXPECT--
+string(4) "seed"
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.