ID: 48745 Updated by: u...@php.net -Summary: Segmentation fault in MySQL extension with mysqlnd Reported By: theta...@php.net -Status: Feedback +Status: Verified Bug Type: MySQL related -Operating System: Solaris 10 x86 +Operating System: All PHP Version: 5.3.0 Assigned To: mysql New Comment:
The trouble goes back to an over optimized COM_LIST_FIELDS communication package and a guess that mysqlnd makes. mysqlnd makes a guess on the number of fields that the deprecated API call mysql_list_fields() may return. The guess is never adjusted to the actual number of fields returned by the Server. As a result mysql_num_fields(), when invoked with a result set handle returned by mysql_list_fields(), may return a faulty number of fields. If a users tries to loop over all fields using the number of fields returned by mysql_num_fields() as a stop condition, he may try to access fields that don't exist. The out-of-bound access is not detected because the internal security check also gets the wrong number of fields reported. Workaround: - don't use deprecated functions, don't use mysql_list_fields().. (there is a reason why we want to deprecate COM_LIST_FIELDS!) - use libmysql until its fixed I don't want to play with the packet decoders and the protocol decoding in the absence of Andrey. I know roughly how to fix, but packet decoding is for Andrey. Andrey is on vacation until August. As its easy to work around, I think it can wait. Previous Comments: ------------------------------------------------------------------------ [2009-06-30 20:10:52] theta...@php.net Here is code to reproduce the bug, it crashes CLI, too: Just create any database and a table in mysql and run this code on it: the problematic function is mysql_list_fields() which is deprecated and does no longer seem to work correct with mysql_field_*() functions. After I cahnged the code in the Contenido database abstraction to instead create a dummy query (select * from table limit 1), like it was done in the Contenido database code for mysqli, it works also with mysql. Maybe, if mysqlnd does not support this, mysql_list_fields() should simply do the same dummy command. <?php $res = array(); mysql_connect('localhost', 'contenido', 'contenido'); $id = mysql_list_fields('contenido', 'con_data'); $count = @mysql_num_fields($id); for ($i=0; $i<$count; $i++) { $res[$i]["table"] = @mysql_field_table ($id, $i); $res[$i]["name"] = @mysql_field_name ($id, $i); $res[$i]["type"] = @mysql_field_type ($id, $i); $res[$i]["len"] = @mysql_field_len ($id, $i); $res[$i]["flags"] = @mysql_field_flags ($id, $i); } mysql_free_result($id); print_r($res); ?> ------------------------------------------------------------------------ [2009-06-30 18:11:17] theta...@php.net Hi Johannes, I am still working on a simple test script. But what I can say is, that when I changed Contenido's config to use mysqli, it does not crash anymore. I found the place where the mysql_field_table is called, but I cannot see any problems there (this is from the included conlib): /* public: return table metadata */ function metadata($table = "", $full = false) { $count = 0; $id = 0; $res = array(); /* * Due to compatibility problems with Table we changed the behavior * of metadata(); * depending on $full, metadata returns the following values: * * - full is false (default): * $result[]: * [0]["table"] table name * [0]["name"] field name * [0]["type"] field type * [0]["len"] field length * [0]["flags"] field flags * * - full is true * $result[]: * ["num_fields"] number of metadata records * [0]["table"] table name * [0]["name"] field name * [0]["type"] field type * [0]["len"] field length * [0]["flags"] field flags * ["meta"][field name] index of field named "field name" * This last one could be used if you have a field name, but no index. * Test: if (isset($result['meta']['myfield'])) { ... */ // if no $table specified, assume that we are working with a query // result if ($table) { $this->connect(); $id = @mysql_list_fields($this->Database, $table); if (!$id) { $this->halt("Metadata query failed."); return false; } } else { $id = $this->Query_ID; if (!$id) { $this->halt("No query specified."); return false; } } $count = @mysql_num_fields($id); // made this IF due to performance (one if is faster than $count if's) if (!$full) { for ($i=0; $i<$count; $i++) { $res[$i]["table"] = @mysql_field_table ($id, $i); $res[$i]["name"] = @mysql_field_name ($id, $i); $res[$i]["type"] = @mysql_field_type ($id, $i); $res[$i]["len"] = @mysql_field_len ($id, $i); $res[$i]["flags"] = @mysql_field_flags ($id, $i); } } else { // full $res["num_fields"]= $count; for ($i=0; $i<$count; $i++) { $res[$i]["table"] = @mysql_field_table ($id, $i); $res[$i]["name"] = @mysql_field_name ($id, $i); $res[$i]["type"] = @mysql_field_type ($id, $i); $res[$i]["len"] = @mysql_field_len ($id, $i); $res[$i]["flags"] = @mysql_field_flags ($id, $i); $res["meta"][$res[$i]["name"]] = $i; } } // free the result only if we were called on a table if ($table) { @mysql_free_result($id); } return $res; } The same code in the other mysqli looks like this and works: /* public: return table metadata */ function metadata($table = "", $full = false) { global $mysqli_type; $count = 0; $id = 0; $res = array (); /* * Due to compatibility problems with Table we changed the behavior * of metadata(); * depending on $full, metadata returns the following values: * * - full is false (default): * $result[]: * [0]["table"] table name * [0]["name"] field name * [0]["type"] field type * [0]["len"] field length * [0]["flags"] field flags * * - full is true * $result[]: * ["num_fields"] number of metadata records * [0]["table"] table name * [0]["name"] field name * [0]["type"] field type * [0]["len"] field length * [0]["flags"] field flags * ["meta"][field name] index of field named "field name" * This last one could be used if you have a field name, but no index. * Test: if (isset($result['meta']['myfield'])) { ... */ // if no $table specified, assume that we are working with a query // result if ($table) { $this->connect(); $id = mysqli_query($this->Link_ID, sprintf("SELECT * FROM %s LIMIT 1", $table)); if (!$id) { $this->halt("Metadata query failed."); return false; } } else { $id = $this->Query_ID; if (!$id) { $this->halt("No query specified."); return false; } } $count = mysqli_num_fields($id); // made this IF due to performance (one if is faster than $count if's) if (!$full) { for ($i = 0; $i < $count; $i ++) { $finfo = mysqli_fetch_field($id); $res[$i]["table"] = $finfo->table; $res[$i]["name"] = $finfo->name; $res[$i]["type"] = $mysqli_type[$finfo->type]; $res[$i]["len"] = $finfo->max_length; $res[$i]["flags"] = $finfo->flags; } } else { // full $res["num_fields"] = $count; for ($i = 0; $i < $count; $i ++) { $finfo = mysqli_fetch_field($id); $res[$i]["table"] = $finfo->table; $res[$i]["name"] = $finfo->name; $res[$i]["type"] = $finfo->type; $res[$i]["len"] = $finfo->max_length; $res[$i]["flags"] = $finfo->flags; $res["meta"][$res[$i]["name"]] = $i; } } // free the result only if we were called on a table if ($table) { mysqli_free_result($id); } return $res; } You can download the whole project Contenido from www.contenido.org (it is quite famous in Germany). Just install and try to login using the default setting (mysql database extension) and mysqlnd installed. Uwe ------------------------------------------------------------------------ [2009-06-30 17:57:31] johan...@php.net Thank you for this bug report. To properly diagnose the problem, we need a short but complete example script to be able to reproduce this bug ourselves. A proper reproducing script starts with <?php and ends with ?>, is max. 10-20 lines long and does not require any external resources such as databases, etc. If the script requires a database to demonstrate the issue, please make sure it creates all necessary tables, stored procedures etc. Please avoid embedding huge scripts into the report. ------------------------------------------------------------------------ [2009-06-30 17:34:58] theta...@php.net During analyzing the core dump, I found the following: (gdb) print mysql_result $5 = (MYSQLND_RES *) 0x0 This is NULL, so ZEND_FETCH_RESOURCE(mysql_result, MYSQL_RES *, &result, -1, "MySQL result", le_result) did not work. Maybe this is the problem? ------------------------------------------------------------------------ [2009-06-30 17:14:14] theta...@php.net I saved the core dump and may supply other variable printouts if needed. Sorry, that I did not found that bug during my RC tests. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/48745 -- Edit this bug report at http://bugs.php.net/?id=48745&edit=1