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

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"