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.

Author: iliaal

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 int 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 1;
+	}
+
+	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 1;
+	}
+	pdo_odbc_stmt_error("SQLBindParameter");
+	return 0;
+}
+
 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 = 0;
+	}
+
 	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 = 1;
 				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, 8, NULL, odbc_free_out_buffer, 0);
+						}
+						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 = 1;
+				return 1;
 			}
 
 			case PDO_PARAM_EVT_EXEC_PRE:

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;

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"

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"