It does not really read past it, but internally it does a strlen() on
the %s argument, which is where the valgrind shows the message.
Technically that code needs to be reviewed, but just from general use
case I think its safer not to pass non-terminated char pointers around.
On 11-Dec-08, at 11:50 AM, Nuno Lopes wrote:
Weird.. Isn't that a bug in php_stream_printf() then? I would say it
shouldn't read past the specified length.. otherwise the same bug
may appear in other places.
What do you think?
Nuno
----- Original Message -----
The patch was already n 5.2, the issue is that the str (key) is not
guaranteed to be NULL terminated (nor does it need to be), so when
strlen() is attempted on top of it you could end up reading more
data then necessary.
On 11-Dec-08, at 10:43 AM, Nuno Lopes wrote:
Modified files: (Branch: PHP_5_3)
/php-src/ext/pdo pdo_stmt.c
Log:
Fixed a possible corruption inside PDOStatement::debugDumpParams()
http://cvs.php.net/viewvc.cgi/php-src/ext/pdo/pdo_stmt.c?r1=1.118.2.38.2.24.2.39&r2=1.118.2.38.2.24.2.40&diff_format=u
Index: php-src/ext/pdo/pdo_stmt.c
diff -u php-src/ext/pdo/pdo_stmt.c:1.118.2.38.2.24.2.39 php-src/
ext/ pdo/pdo_stmt.c:1.118.2.38.2.24.2.40
@@ -2209,7 +2209,9 @@
if (res == HASH_KEY_IS_LONG) {
php_stream_printf(out TSRMLS_CC, "Key: Position #%ld:\n", num);
} else if (res == HASH_KEY_IS_STRING) {
- php_stream_printf(out TSRMLS_CC, "Key: Name: [%d] %.*s\n",
len, len, str);
+ char *s = estrndup(str, len);
+ php_stream_printf(out TSRMLS_CC, "Key: Name: [%d] %.*s\n",
len, len, s);
+ efree(s);
}
Sorry for my ignorance, but isn't the new code exactly equivalent
to the old one, albeit a bit slower? I can't really see how a
strndup() can fix a corruption there.. If there's some problem,
probably it's deeper than this..
Nuno
Ilia Alshanetsky
Ilia Alshanetsky
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php