php_mysql_fetch_hash writes long value into int

Bug #52636	php_mysql_fetch_hash writes long value into int
Submitted:	2010-08-18 16:35 UTC	Modified:	2010-08-18 22:02 UTC
From:	rein at basefarm dot no	Assigned:	kalle (profile)
Status:	Closed	Package:	MySQL related
PHP Version:	5.3.3	OS:	Solaris10 on sparc
Private report:	No	CVE-ID:	None

[2010-08-18 16:35 UTC] rein at basefarm dot no

Description:
------------
In php_mysql_fetch_hash(), zend_parse_parameters() is passed &result_type, an int variable, which it is instructed to write a long value into.  On a big-endian system like the sparc it writes outside the memory location of result_type.

The mysql_fetch_array tests in:

 ext/mysql/tests/002.phpt
 ext/mysql/tests/mysql_fetch_array.phpt

fails on Solaris sparc systems without this patch, it always behaves as if the one-argument version of mysql_fetch_array is called.

This bug may be causing the problem reported in bug#51601.

Patches

du (last revision 2010-12-28 06:58 UTC by duwq at ifeng dot com)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports

[2010-08-18 16:43 UTC] rein at basefarm dot no

Adding a patch doesn't work, so here it is:

Index: ext/mysql/php_mysql.c
===================================================================
RCS file: ext/mysql/php_mysql.c,v
retrieving revision 1.1.1.21
retrieving revision 1.2
diff -u -u -r1.1.1.21 -r1.2
--- ext/mysql/php_mysql.c	16 Aug 2010 17:58:34 -0000	1.1.1.21
+++ ext/mysql/php_mysql.c	18 Aug 2010 14:09:13 -0000	1.2
@@ -2040,9 +2040,12 @@
 	} else
 #endif
 	{
-		if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "r|l", &res, &result_type) == FAILURE) {
+		long res_type = result_type;
+
+		if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "r|l", &res, &res_type) == FAILURE) {
 			return;
 		}
+		result_type = res_type;
 		if (!result_type) {
 			/* result_type might have been set outside, so only overwrite when not set */
 			result_type = MYSQL_BOTH;

[2010-08-18 19:54 UTC] kalle@php.net

-Status: Open +Status: Assigned -Assigned To: +Assigned To: kalle

[2010-08-18 19:54 UTC] kalle@php.net

Hi, wouldn't it be easier if we just changed the result_type to be a long in the prototype?

[2010-08-18 20:56 UTC] rein at basefarm dot no

Yes, probably, thought about that I too.  I discarded it as I just don´t like to change argument types unless I´m absolutely sure it doesn´t have any unforseen side-effects.  And I don´t consider my limited knowledge of php internals to be good enough for that.  Though, being a static function it should be pretty safe here..

[2010-08-18 21:06 UTC] kalle@php.net

Just my thought, could try try it out if it works, if not then I guess we need to suffix the MYSQL_FETCH defines with L to make them a long.

Either way, reply back with your results and ill commit the fix tonight, thanks :)

[2010-08-18 21:32 UTC] rein at basefarm dot no

Using the prototype change, all mysql tests succeed for me on solaris10/sparc, solaris10/x86_64 and redhat4/x86_64 :-)  So from my point of view it is go ahead with the prototype variant.

[2010-08-18 22:02 UTC] kalle@php.net

-Status: Assigned +Status: Closed

[2010-08-18 22:02 UTC] kalle@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.