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

Reply via email to