From 08073b06581c6dc9234cfb2e9b187a598154c8ab Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 27 Dec 2019 13:20:11 +0100 Subject: [PATCH] Fix #79038: PDOStatement::nextRowset() leaks column values Firstly, we must not rely on `stmt->column_count` when freeing the driver specific column values, but rather store the column count in the driver data. Since the column count is a `short`, 16 bit are sufficient, so we can store it in reserved bits of `pdo_odbc_stmt`. Furthermore, we must not allocate new column value storage when the statement is not executed, but rather when the column value storage has not been allocated. Finally, we have to introduce a driver specific `cursor_closer` to avoid that `::closeCursor()` calls `odbc_stmt_next_rowset()` which then frees the column value storage, because it may be still needed for bound columns. --- NEWS | 3 +++ ext/pdo_odbc/odbc_stmt.c | 24 +++++++++++++++++++----- ext/pdo_odbc/php_pdo_odbc_int.h | 3 ++- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 29d0f3d9aff..a6c9cbf2b57 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,9 @@ PHP NEWS . Fixed bug #79188 (Memory corruption in preg_replace/preg_replace_callback and unicode). (Nikita) +- PDO_ODBC: + . Fixed bug #79038 (PDOStatement::nextRowset() leaks column values). (cmb) + - Standard: . Fixed bug #79254 (getenv() w/o arguments not showing changes). (cmb) diff --git a/ext/pdo_odbc/odbc_stmt.c b/ext/pdo_odbc/odbc_stmt.c index efcc9121df6..08a08b2a54b 100644 --- a/ext/pdo_odbc/odbc_stmt.c +++ b/ext/pdo_odbc/odbc_stmt.c @@ -126,13 +126,14 @@ static void free_cols(pdo_stmt_t *stmt, pdo_odbc_stmt *S) if (S->cols) { int i; - for (i = 0; i < stmt->column_count; i++) { + for (i = 0; i < S->col_count; i++) { if (S->cols[i].data) { efree(S->cols[i].data); } } efree(S->cols); S->cols = NULL; + S->col_count = 0; } } @@ -262,14 +263,14 @@ static int odbc_stmt_execute(pdo_stmt_t *stmt) SQLRowCount(S->stmt, &row_count); stmt->row_count = row_count; - if (!stmt->executed) { + if (S->cols == NULL) { /* do first-time-only definition of bind/mapping stuff */ SQLSMALLINT colcount; /* how many columns do we have ? */ SQLNumResultCols(S->stmt, &colcount); - stmt->column_count = (int)colcount; + stmt->column_count = S->col_count = (int)colcount; S->cols = ecalloc(colcount, sizeof(pdo_odbc_column)); S->going_long = 0; } @@ -847,13 +848,25 @@ static int odbc_stmt_next_rowset(pdo_stmt_t *stmt) free_cols(stmt, S); /* how many columns do we have ? */ SQLNumResultCols(S->stmt, &colcount); - stmt->column_count = (int)colcount; + stmt->column_count = S->col_count = (int)colcount; S->cols = ecalloc(colcount, sizeof(pdo_odbc_column)); S->going_long = 0; return 1; } +static int odbc_stmt_close_cursor(pdo_stmt_t *stmt) +{ + SQLRETURN rc; + pdo_odbc_stmt *S = (pdo_odbc_stmt*)stmt->driver_data; + + rc = SQLCloseCursor(S->stmt); + if (rc != SQL_SUCCESS && rc != SQL_SUCCESS_WITH_INFO) { + return 0; + } + return 1; +} + const struct pdo_stmt_methods odbc_stmt_methods = { odbc_stmt_dtor, odbc_stmt_execute, @@ -864,7 +877,8 @@ const struct pdo_stmt_methods odbc_stmt_methods = { odbc_stmt_set_param, odbc_stmt_get_attr, /* get attr */ NULL, /* get column meta */ - odbc_stmt_next_rowset + odbc_stmt_next_rowset, + odbc_stmt_close_cursor }; /* diff --git a/ext/pdo_odbc/php_pdo_odbc_int.h b/ext/pdo_odbc/php_pdo_odbc_int.h index a7812764b31..28717f27729 100644 --- a/ext/pdo_odbc/php_pdo_odbc_int.h +++ b/ext/pdo_odbc/php_pdo_odbc_int.h @@ -151,7 +151,8 @@ typedef struct { zend_ulong convbufsize; unsigned going_long:1; unsigned assume_utf8:1; - unsigned _spare:30; + signed col_count:16; + unsigned _spare:14; } pdo_odbc_stmt; typedef struct {