Skip to content

Commit 08073b0

Browse files
committed
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.
1 parent 16c7c71 commit 08073b0

File tree

3 files changed

+24
-6
lines changed

3 files changed

+24
-6
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ PHP NEWS
1818
. Fixed bug #79188 (Memory corruption in preg_replace/preg_replace_callback
1919
and unicode). (Nikita)
2020

21+
- PDO_ODBC:
22+
. Fixed bug #79038 (PDOStatement::nextRowset() leaks column values). (cmb)
23+
2124
- Standard:
2225
. Fixed bug #79254 (getenv() w/o arguments not showing changes). (cmb)
2326

ext/pdo_odbc/odbc_stmt.c

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,14 @@ static void free_cols(pdo_stmt_t *stmt, pdo_odbc_stmt *S)
126126
if (S->cols) {
127127
int i;
128128

129-
for (i = 0; i < stmt->column_count; i++) {
129+
for (i = 0; i < S->col_count; i++) {
130130
if (S->cols[i].data) {
131131
efree(S->cols[i].data);
132132
}
133133
}
134134
efree(S->cols);
135135
S->cols = NULL;
136+
S->col_count = 0;
136137
}
137138
}
138139

@@ -262,14 +263,14 @@ static int odbc_stmt_execute(pdo_stmt_t *stmt)
262263
SQLRowCount(S->stmt, &row_count);
263264
stmt->row_count = row_count;
264265

265-
if (!stmt->executed) {
266+
if (S->cols == NULL) {
266267
/* do first-time-only definition of bind/mapping stuff */
267268
SQLSMALLINT colcount;
268269

269270
/* how many columns do we have ? */
270271
SQLNumResultCols(S->stmt, &colcount);
271272

272-
stmt->column_count = (int)colcount;
273+
stmt->column_count = S->col_count = (int)colcount;
273274
S->cols = ecalloc(colcount, sizeof(pdo_odbc_column));
274275
S->going_long = 0;
275276
}
@@ -847,13 +848,25 @@ static int odbc_stmt_next_rowset(pdo_stmt_t *stmt)
847848
free_cols(stmt, S);
848849
/* how many columns do we have ? */
849850
SQLNumResultCols(S->stmt, &colcount);
850-
stmt->column_count = (int)colcount;
851+
stmt->column_count = S->col_count = (int)colcount;
851852
S->cols = ecalloc(colcount, sizeof(pdo_odbc_column));
852853
S->going_long = 0;
853854

854855
return 1;
855856
}
856857

858+
static int odbc_stmt_close_cursor(pdo_stmt_t *stmt)
859+
{
860+
SQLRETURN rc;
861+
pdo_odbc_stmt *S = (pdo_odbc_stmt*)stmt->driver_data;
862+
863+
rc = SQLCloseCursor(S->stmt);
864+
if (rc != SQL_SUCCESS && rc != SQL_SUCCESS_WITH_INFO) {
865+
return 0;
866+
}
867+
return 1;
868+
}
869+
857870
const struct pdo_stmt_methods odbc_stmt_methods = {
858871
odbc_stmt_dtor,
859872
odbc_stmt_execute,
@@ -864,7 +877,8 @@ const struct pdo_stmt_methods odbc_stmt_methods = {
864877
odbc_stmt_set_param,
865878
odbc_stmt_get_attr, /* get attr */
866879
NULL, /* get column meta */
867-
odbc_stmt_next_rowset
880+
odbc_stmt_next_rowset,
881+
odbc_stmt_close_cursor
868882
};
869883

870884
/*

ext/pdo_odbc/php_pdo_odbc_int.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ typedef struct {
151151
zend_ulong convbufsize;
152152
unsigned going_long:1;
153153
unsigned assume_utf8:1;
154-
unsigned _spare:30;
154+
signed col_count:16;
155+
unsigned _spare:14;
155156
} pdo_odbc_stmt;
156157

157158
typedef struct {

0 commit comments

Comments
 (0)