php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #48745 mysqlnd: mysql_num_fields returns wrong column count for mysql_list_fields
Submitted: 2009-06-30 17:08 UTC Modified: 2009-09-09 21:53 UTC
Votes:1
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:0 (0.0%)
Same OS:1 (100.0%)
From: thetaphi@php.net Assigned: mysql (profile)
Status: Closed Package: MySQL related
PHP Version: 5.3SVN-2009-08-29 OS: *
Private report: No CVE-ID: None
 [2009-06-30 17:08 UTC] thetaphi@php.net
Description:
------------
Installed PHP 5.3 today on our Solaris server running with NSAPI as SAPI module (which is not the problem here). Our test environment with some applications like MediaWiki and our own PHP scripts worked as exspected.

We are using the new mysqlnd, because under solaris 10 with Blastwave mysql libs you have problems with compiling (libs are 32/64 bit dual, mysql_config only return 64 bit params, php should be compiled as 32bit, see my php_dev mail after RC4). Mysqlnd works super, mediawiki runs much faster than before.

With one application, which was not tested before, we have a problem. The content managment system Contenido 4.8.12 (www.condenido.org) works in the frontend without problem, so the website is running, but the backend crashes PHP with an SIGSEGV. The stacktrace is attached.

Contenido uses the old mysql extension (which also uses mysqlnd).

Reproduce code:
---------------
I do not exectly know at which portion of contenido's code it crashes. It seems that Z_STRLEN_P(return_value) = strlen(mysql_field->table) produces a sigsegv (mysql_field->table == NULL):


Expected result:
----------------
It should not crash the webserver process.

Actual result:
--------------
Program terminated with signal 11, Segmentation fault.
#0  0xfc3925e2 in php_mysql_field_info (ht=0, return_value=0xa5b157c, return_value_ptr=0x0, this_ptr=0x0, return_value_used=1, 
    tsrm_ls=0xa61f228, entry_type=2) at /pangaea/install/php-5.3.0/ext/mysql/php_mysql.c:2410
2410                            Z_STRLEN_P(return_value) = strlen(mysql_field->table);

(gdb) where
#0  0xfc3925e2 in php_mysql_field_info (ht=0, return_value=0xa5b157c, return_value_ptr=0x0, this_ptr=0x0, return_value_used=1, 
    tsrm_ls=0xa61f228, entry_type=2) at /pangaea/install/php-5.3.0/ext/mysql/php_mysql.c:2410
#1  0xfc56ce5d in zend_do_fcall_common_helper_SPEC (execute_data=0xa406f20, tsrm_ls=0xa1315e0)
    at /pangaea/install/php-5.3.0/Zend/zend_vm_execute.h:313
#2  0xfc56bce2 in execute (op_array=0xa511654, tsrm_ls=0xa1315e0) at /pangaea/install/php-5.3.0/Zend/zend_vm_execute.h:104
#3  0xfc54a103 in zend_execute_scripts (type=8, tsrm_ls=0xa1315e0, retval=0x0, file_count=3)
    at /pangaea/install/php-5.3.0/Zend/zend.c:1188
#4  0xfc4f5562 in php_execute_script (primary_file=0xebbe7cb8, tsrm_ls=0xa1315e0) at /pangaea/install/php-5.3.0/main/main.c:2196
#5  0xfc5d5916 in php5_execute (pb=0x82efe08, sn=0x9b9939c, rq=0x9b99414) at /pangaea/install/php-5.3.0/sapi/nsapi/nsapi.c:1040
#6  0xfecfb147 in func_exec_str () from /pangaea/webserver70/lib/libns-httpd40.so
#7  0xfecfbd2a in INTfunc_exec_directive () from /pangaea/webserver70/lib/libns-httpd40.so
#8  0xfed009d6 in INTservact_service () from /pangaea/webserver70/lib/libns-httpd40.so
#9  0xfed01a39 in INTservact_handle_processed () from /pangaea/webserver70/lib/libns-httpd40.so
#10 0xfed5e358 in __1cLHttpRequestUUnacceleratedRespond6M_v_ () from /pangaea/webserver70/lib/libns-httpd40.so
#11 0xfed5d5ba in __1cLHttpRequestNHandleRequest6MpnGnetbuf_I_i_ () from /pangaea/webserver70/lib/libns-httpd40.so
#12 0xfed5be90 in __1cNDaemonSessionDrun6M_v_ () from /pangaea/webserver70/lib/libns-httpd40.so
#13 0xfeb861fc in ThreadMain () from /pangaea/webserver70/lib/libnsprwrap.so
#14 0xfe0bb6c9 in _pt_root () from /pangaea/webserver70/lib/libnspr4.so
#15 0xfd37fd36 in _thr_setup () from /lib/libc.so.1
#16 0xfd380020 in L3_doit () from /lib/libc.so.1
#17 0xfb2d0400 in ?? ()
#18 0x00000000 in ?? ()

(gdb) print *mysql_field
$2 = {name = 0x0, org_name = 0x0, table = 0x0, org_table = 0x0, db = 0x0, catalog = 0x0, def = 0x0, length = 0, max_length = 0, 
  name_length = 0, org_name_length = 0, table_length = 0, org_table_length = 0, db_length = 0, catalog_length = 0, def_length = 0, 
  flags = 0, decimals = 0, charsetnr = 0, type = MYSQL_TYPE_DECIMAL, root = 0x0, root_len = 0}


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2009-06-30 17:14 UTC] thetaphi@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.
 [2009-06-30 17:34 UTC] thetaphi@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:57 UTC] johannes@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 18:11 UTC] thetaphi@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 20:10 UTC] thetaphi@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-07-01 16:54 UTC] uw@php.net
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.
 [2009-07-01 17:14 UTC] thetaphi@php.net
As I noted before: This is not my code and there may be a lot of users installing software using the old mysql extension and also this function.

For me the workaround was to enable the mysqli configuration option in Contenido. The only problem is, that it is not enabled per default in this CMS (although it says: PHP 5 only). Contenido uses a PHP-written database abstraction (from where I copied the functions in this bug report) and it was easy to exchange by the other one. It was also fixable by doing a dummy query "select * from table limit 1" and looking into the field descriptions for the result set (this is how the abstraction layer fr mysqli does it in Contenido).

So many thanks for looking at it, I will post a note in the Contenido mailing list about this problem and the workaround.
 [2009-07-02 07:01 UTC] uw@php.net
Thanks. As said, its fixable, I know the cause, I may know a hack to fix it but the call is deprecated.  There are zillions of MySQL 4.0 users but we don't support MySQL 4.0 any more. There may be many list_fields users, but list_fields is deprecated. 

At some point you simply have to stop giving old, deprecated calls the highest priority.

Apart from that, I don't want to hack with the packet decoders in the absence of Andrey if the workaround is as easy as: compile ext/mysql against libmysql (like ever since) and try again in four weeks.

Ulf
 [2009-07-02 07:19 UTC] thetaphi@php.net
Thanks! I understand the problem and that it is deprecated. The important thing is: it should *not* SIGSEGV. So the best idea would be to simply disable the whole function, if it is not working with mysqlnd and you are not willing to support it (something like: "deprecated functions work with libmysqlclient but not with mysqlnd". They should simply return false or throw an error or should removed at all). Because of the sigsegv it was hard to find out, where the error really occurred in this thousands of lines of foreign PHP code.

But if Andrew is able to fix it, let's wait for it.

> compile ext/mysql against libmysql (like ever since) and try
> again in four weeks

My problem was, that this does not work easily with PHP 32bit on Solaris x64 using /opt/csw libs (mysql_config only returns 64bit libs, includes strange not GCC compatible CFLAGS and so on). But this is another problem. It was also broken in 5.2 (but you were able to fix it), but with 5.3 it now produces hard compilation errors (and can only be fixed by replacing mysql_config with a "dummy" that returns correct CFLAGS and LIB paths). But Linux users can always compile against libmysql this.

For Solaris users mysqlnd is the best for problem-less installation!

The easiest workaround was to use mysqli in our case :) For me this bug is obsolete, I only want to keep it open because of the SIGSEGV.

Uwe
 [2009-08-28 09:30 UTC] svn@php.net
Automatic comment from SVN on behalf of andrey
Revision: http://svn.php.net/viewvc/?view=revision&revision=287834
Log: Fix for bug#48745
mysqlnd: mysql_num_fields returns wrong column count for mysql_list_fields
 [2009-08-28 09:38 UTC] andrey@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

Uwe, the fix will appear in 5.3.1 
 [2009-08-29 12:46 UTC] thetaphi@php.net
I still get SIGSEGV when logging into to the CMS Contenido with mysql extension instead of mysqli. I will reopen the bug report after investigating and analyzing the core dump (maybe its a new bug).
 [2009-08-29 13:11 UTC] thetaphi@php.net
It is still the same stack trace:

Program terminated with signal 11, Segmentation fault.
#0  0xfc3931aa in php_mysql_field_info (ht=0, return_value=0xd7db5a8, return_value_ptr=0x0, this_ptr=0x0, return_value_used=1, 
    tsrm_ls=0xc96f228, entry_type=2) at /pangaea/install/php5.3-200908291030/ext/mysql/php_mysql.c:2410
2410                            Z_STRLEN_P(return_value) = strlen(mysql_field->table);
(gdb) where
#0  0xfc3931aa in php_mysql_field_info (ht=0, return_value=0xd7db5a8, return_value_ptr=0x0, this_ptr=0x0, return_value_used=1, 
    tsrm_ls=0xc96f228, entry_type=2) at /pangaea/install/php5.3-200908291030/ext/mysql/php_mysql.c:2410
#1  0xfc56d291 in zend_do_fcall_common_helper_SPEC (execute_data=0xc6ddf20, tsrm_ls=0xc404e20)
    at /pangaea/install/php5.3-200908291030/Zend/zend_vm_execute.h:313
#2  0xfc56c116 in execute (op_array=0xc7dcaec, tsrm_ls=0xc404e20) at /pangaea/install/php5.3-200908291030/Zend/zend_vm_execute.h:104
#3  0xfc54a437 in zend_execute_scripts (type=8, tsrm_ls=0xc404e20, retval=0x0, file_count=3)
    at /pangaea/install/php5.3-200908291030/Zend/zend.c:1188
#4  0xfc4f58d6 in php_execute_script (primary_file=0xe7de7cb8, tsrm_ls=0xc404e20)
    at /pangaea/install/php5.3-200908291030/main/main.c:2212
#5  0xfc5d60ea in php5_execute (pb=0x81ae228, sn=0xc2661dc, rq=0xc266254)
    at /pangaea/install/php5.3-200908291030/sapi/nsapi/nsapi.c:1047
#6  0xfecfb147 in func_exec_str () from /pangaea/webserver70/lib/libns-httpd40.so
#7  0xfecfbd2a in INTfunc_exec_directive () from /pangaea/webserver70/lib/libns-httpd40.so
#8  0xfed009d6 in INTservact_service () from /pangaea/webserver70/lib/libns-httpd40.so
#9  0xfed01a39 in INTservact_handle_processed () from /pangaea/webserver70/lib/libns-httpd40.so
#10 0xfed5e358 in __1cLHttpRequestUUnacceleratedRespond6M_v_ () from /pangaea/webserver70/lib/libns-httpd40.so
#11 0xfed5d5ba in __1cLHttpRequestNHandleRequest6MpnGnetbuf_I_i_ () from /pangaea/webserver70/lib/libns-httpd40.so
#12 0xfed5be90 in __1cNDaemonSessionDrun6M_v_ () from /pangaea/webserver70/lib/libns-httpd40.so
#13 0xfeb861fc in ThreadMain () from /pangaea/webserver70/lib/libnsprwrap.so
#14 0xfe0bb6c9 in _pt_root () from /pangaea/webserver70/lib/libnspr4.so
#15 0xfd37fd36 in _thr_setup () from /lib/libc.so.1
#16 0xfd380020 in L3_doit () from /lib/libc.so.1
#17 0xeb9d3c00 in ?? ()
#18 0x00000000 in ?? ()
(gdb) print *mysql_field
$2 = {name = 0x0, org_name = 0x0, table = 0x0, org_table = 0x0, db = 0x0, catalog = 0x0, def = 0x0, length = 0, max_length = 0, 
  name_length = 0, org_name_length = 0, table_length = 0, org_table_length = 0, db_length = 0, catalog_length = 0, def_length = 0, 
  flags = 0, decimals = 0, charsetnr = 0, type = MYSQL_TYPE_DECIMAL, root = 0x0, root_len = 0}
(gdb) print mysql_result
$3 = (MYSQLND_RES *) 0x0
(gdb) 

The new version is installed (I checked the snaps.php.net version for your changes).

The attached PHP scipt to reproduce generates similar stack trace:

Core was generated by `php test.php'.
Program terminated with signal 11, Segmentation fault.
#0  0x081a24f2 in php_mysql_field_info (ht=0, return_value=0x887e28c, return_value_ptr=0x0, this_ptr=0x0, return_value_used=1, 
    tsrm_ls=0x8b057d8, entry_type=2) at /pangaea/install/php5.3-200908291030/ext/mysql/php_mysql.c:2410
2410                            Z_STRLEN_P(return_value) = strlen(mysql_field->table);
(gdb) where
#0  0x081a24f2 in php_mysql_field_info (ht=0, return_value=0x887e28c, return_value_ptr=0x0, this_ptr=0x0, return_value_used=1, 
    tsrm_ls=0x8b057d8, entry_type=2) at /pangaea/install/php5.3-200908291030/ext/mysql/php_mysql.c:2410
#1  0x0837c5d9 in zend_do_fcall_common_helper_SPEC (execute_data=0x8abb468, tsrm_ls=0x885ecc0)
    at /pangaea/install/php5.3-200908291030/Zend/zend_vm_execute.h:313
#2  0x0837b45e in execute (op_array=0x886e340, tsrm_ls=0x885ecc0) at /pangaea/install/php5.3-200908291030/Zend/zend_vm_execute.h:104
#3  0x0835977f in zend_execute_scripts (type=8, tsrm_ls=0x885ecc0, retval=0x0, file_count=3)
    at /pangaea/install/php5.3-200908291030/Zend/zend.c:1188
#4  0x08304c1e in php_execute_script (primary_file=0x8047c90, tsrm_ls=0x885ecc0)
    at /pangaea/install/php5.3-200908291030/main/main.c:2212
#5  0x083e5406 in main (argc=2, argv=0x8047d24) at /pangaea/install/php5.3-200908291030/sapi/cli/php_cli.c:1188

$ php --version
PHP 5.3.1-dev (cli) (built: Aug 29 2009 14:35:51) 
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2009 Zend Technologies

From this snap: php5.3-200908291030.tar.bz2
Uwe
 [2009-09-09 17:14 UTC] uw@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

Next attempt... I hope this time its really fixed :). Please try again, thanks!
 [2009-09-09 21:53 UTC] thetaphi@php.net
Hi,

works fine, bug fixed! Thanks!

Uwe
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Mar 28 10:01:26 2024 UTC