php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #70351 zend_parse_parameters returns invalid address for string parameter on 64bit OS
Submitted: 2015-08-25 05:21 UTC Modified: 2015-08-25 05:45 UTC
From: chenxy at gmail dot com Assigned:
Status: Wont fix Package: Reproducible crash
PHP Version: 7.0.0RC1 OS: CentOS 7.1.1513 x86_64
Private report: No CVE-ID: None
Have you experienced this issue?
Rate the importance of this bug to you:

 [2015-08-25 05:21 UTC] chenxy at gmail dot com
Description:
------------
I am modifying an old extension of PHP 5.x, trying to let it to work on PHP 7.0.0.  But I always get 'Segmentation fault'.  zend_parse_parameters always returns an invalid address for the char *key.

My OS is CentOS 7.1.1503, x86_64.
PHP version is 7.0.0beta[123] and 7.0.0RC1, compiled with default options.

The code is as following:

PHP_METHOD(TestMod, get)
{
	char *key = NULL;
	// Strictly, I should use 'size_t key_len', but 'int key_len' works properly for PHP 5.x
	int   key_len = 0;

	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &key, &key_len) == FAILURE) {
		return;
	}

	printf("key: %s\n", key);
	RETURN_TRUE;
}

After a long time of debugging, I find that it's due to the declaration of 'key_len', if I use 'size_t key_len', the extension runs normally, but 'int key_len' always causes an invalid pointer returned.


PHP 7.0.0 uses zend_string internally in processing string parameters.  It returns ZSTR_LEN(str) as the string length:

#define ZSTR_LEN(zstr)  (zstr)->len
...
                *dest = ZSTR_VAL(str);
                *dest_len = ZSTR_LEN(str);

(zstr)->len is of type 'size_t', which is 8 bytes in an OS of x86_64.

But in PHP 5.x, zval is used internally in processing string parameters.  it returns (zval).value.str.len as the string length:

#define Z_STRLEN_PP(zval_pp)    Z_STRLEN(**zval_pp)
#define Z_STRLEN(zval)                  (zval).value.str.len
...
                                                *p = Z_STRVAL_PP(arg);
                                                *pl = Z_STRLEN_PP(arg);

(zval).value.str.len is of type 'int', which is 4 bytes in an OS of x86_64.

So if I use 'int key_len' as input, in PHP 5.x:
	*dest_len = (zval).value.str.len;
But in PHP 7.0.0:
	*dest_len = (zstr)->len;

So in PHP 7.0.0, *dest_len overwrites the content of *dest, causes it to point to an invalid address, which leads to 'Segmentation Fault'.



Test script:
---------------
PHP_METHOD(TestMod, get)
{
	char *key = NULL;
	// Strictly, I should use 'size_t key_len', but 'int key_len' works properly for PHP 5.x
	int   key_len = 0;

	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &key, &key_len) == FAILURE) {
		return;
	}

	printf("key: %s\n", key);
	RETURN_TRUE;
}


Expected result:
----------------
A valid address of char *key should be returned.

Although 'size_t key_len' works normally, but the old fashion of 'int key_len' causes a crash, which is hard to debug.

Checking of this type of improper declaration in compilation time may be difficult.  Perhaps we may use sizeof() to check the size of the input variable.  If it is less than 8, we may throw an exception.

Anyway, programmers hate 'segmentation fault'.

Actual result:
--------------
See the debugging process to clarify it:


(gdb) list
1128		}
1129		if (check_null && UNEXPECTED(!str)) {
1130			*dest = NULL;
1131			*dest_len = 0;
1132		} else {
1133			*dest = ZSTR_VAL(str);
1134			*dest_len = ZSTR_LEN(str);
1135		}
1136		return 1;
1137	}
(gdb) print dest
$1 = (char **) 0x7fffffffa658
(gdb) print *dest
$2 = 0x0
(gdb) print dest_len
$3 = (size_t *) 0x7fffffffa654
(gdb) s
1134			*dest_len = ZSTR_LEN(str);
(gdb) print dest
$4 = (char **) 0x7fffffffa658
(gdb) print *dest
$5 = 0x7ffff5e555d8 "abc"
(gdb) s
1136		return 1;
(gdb) print dest_len
$6 = (size_t *) 0x7fffffffa654
(gdb) print *dest_len
$7 = 3
(gdb) print dest
$8 = (char **) 0x7fffffffa658
(gdb) print *dest
$9 = 0x7fff00000000 <Address 0x7fff00000000 out of bounds>
(gdb) 


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-08-25 05:43 UTC] requinix@php.net
-Status: Open +Status: Wont fix
 [2015-08-25 05:43 UTC] requinix@php.net
This change was documented in UPGRADING.

https://github.com/php/php-src/blob/PHP-7.0.0/UPGRADING.INTERNALS#L75
> The 's' spec expects parameters of the type char * and size_t, no int anymore.

Be sure to read through the rest of it to avoid other unexpected problems.
 [2015-08-25 05:45 UTC] requinix@php.net
*UPGRADING.INTERNALS, obviously.
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Sun Mar 07 00:01:23 2021 UTC