Bug #67130 [Asn->Fbk]: nextRowset causes memory corruption
Edit report at https://bugs.php.net/bug.php?id=67130&edit=1
ID: 67130
Updated by: ssufficool@php.net
Reported by: kaido at tradenet dot ee
Summary: nextRowset causes memory corruption
-Status: Assigned
+Status: Feedback
Type: Bug
Package: PDO DBlib
Operating System: linux
PHP Version: master-Git-2014-04-24 (Git)
Assigned To: ssufficool
Block user comment: N
Private report: N
New Comment:
Please try using this snapshot:
http://snaps.php.net/php-trunk-latest.tar.gz
For Windows:
http://windows.php.net/snapshots/
Fixed along with Bug #64511
Previous Comments:
------------------------------------------------------------------------
[2014-10-21 03:57:11] ssufficool@php.net
Instead of strdup I would null the col name pointers to (hopefully) cause pdo core to skip the
deallocate. I would hate to introduce an ever so slight performance penalty with a strdup if not
*absolutely* needed.
------------------------------------------------------------------------
[2014-04-26 08:29:14] kaido at tradenet dot ee
the problem/bug is as follows:
(1) ext/pdo/pdo_stmt.c:pdo_stmt_do_next_rowset() frees column names with efree()
(2) column names are set in ext/pdo_dblib/dblib_stmt.c:pdo_dblib_stmt_describe() with direct call to
dbcolname(). the returned name points to a fixed structure and as such does not need to be freed
later.
(3) ext/pdo_dblib/dblib_stmt.c:pdo_dblib_stmt_cursor_closer() and
ext/pdo_dblib/dblib_stmt.c:pdo_dblib_stmt_stmt_dtor() know the fact (2) and do not free column names
(4) it seems that pdo expects the column names to be allocated - other drivers (oci, mysql etc) do
it this way.
so the proposed solution/fix would be:
(a) in ext/pdo_dblib/dblib_stmt.c:pdo_dblib_stmt_describe() estrdup() the column name as follows:
col->name = (char*)estrdup(dbcolname(H->link, colno+1));
b) add the code:
struct pdo_column_data *cols = stmt->columns;
int i;
for (i = 0; i < stmt->column_count; i++) {
efree(cols[i].name);
}
into ext/pdo_dblib/dblib_stmt.c:pdo_dblib_stmt_cursor_closer() and
ext/pdo_dblib/dblib_stmt.c:pdo_dblib_stmt_stmt_dtor() to free column names in those code paths.
Can someone with better knowledge of the code here take a look, confirm my observations, and commit
the fix, please.
------------------------------------------------------------------------
[2014-04-24 20:24:09] kaido at tradenet dot ee
Description:
------------
php:
PHP 5.4.29-dev (cli) (built: Apr 24 2014 16:49:28) (DEBUG)
configured with:
./configure --enable-debug --with-mssql --with-pdo-dblib --enable-mbstring
calling PDOStatement::nextRowset() corrupts memory.
Test script:
---------------
replace server/database names and login credentials.
freetds.conf has 'tds version = 8.0' as only item in 'global' section.
<?php
$dsn = "dblib:host=xxxxx.xxxx.xxx;dbname=xxx";
$pdo = new PDO($dsn, 'user', 'password');
$stmt = $pdo->query('select 1');
$stmt->fetch();
$stmt->nextRowset();
?>
Actual result:
--------------
[Thu Apr 24 23:19:09 2014] Script: '/webroot/k2/ekl/b1.php'
---------------------------------------
/root/php/php-src/ext/pdo/pdo_stmt.c(2074) : Block 0x03058a48 status:
Beginning: Freed (magic=0x00000000, expected=0x99954317)
Start: Overflown (magic=0x00000000 instead of 0x541E9F24)
At least 4 bytes overflown
End: Overflown (magic=0x00000000 instead of 0x546AF508)
At least 4 bytes overflown
---------------------------------------
Warning: PDOStatement::nextRowset(): SQLSTATE[HY000]: General error: PDO_DBLIB: dbresults() returned
FAIL in /webroot/k2/ekl/b1.php on line 10
[Thu Apr 24 23:19:09 2014] Script: '/webroot/k2/ekl/b1.php'
/root/php/php-src/ext/pdo_dblib/pdo_dblib.c(113) : Freeing 0x7F109142F9C0 (8 bytes),
script=/webroot/k2/ekl/b1.php
[Thu Apr 24 23:19:09 2014] Script: '/webroot/k2/ekl/b1.php'
/root/php/php-src/ext/pdo_dblib/pdo_dblib.c(118) : Freeing 0x7F1091431490 (73 bytes),
script=/webroot/k2/ekl/b1.php
=== Total 2 memory leaks detected ===
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=67130&edit=1
Thread (9 messages)