php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #76167 mbstring may use/efree() pointer from some previous request
Submitted: 2018-03-30 11:25 UTC Modified: 2021-10-21 11:53 UTC
Votes:2
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:0 (0.0%)
Same OS:1 (100.0%)
From: cataphract@php.net Assigned: cmb (profile)
Status: Closed Package: mbstring related
PHP Version: 5.6.35 OS: irrelevant
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: cataphract@php.net
New email:
PHP Version: OS:

 

 [2018-03-30 11:25 UTC] cataphract@php.net
Description:
------------
mbstring seems to assume that calls to rinit are matched with calls to rshutdown, but this may not be case. In particular, if another extension fails its rinit (after mbstring has run its), either by returning FAILURE or calling zend_error(), then rshutdown will never be called, as PG(modules_activated) will be 0.

when mbstring_globals.current_detect_order_list is allocated a pointer, this pointer is freed in rshutdown:

PHP_RSHUTDOWN_FUNCTION(mbstring)
{
	const struct mb_overload_def *p;
	zend_function *orig;

	if (MBSTRG(current_detect_order_list) != NULL) {
		efree(MBSTRG(current_detect_order_list));
		MBSTRG(current_detect_order_list) = NULL;
		MBSTRG(current_detect_order_list_size) = 0;
	}

At this point, the pointer stored by current_detect_order_list may not be valid anymore, because it may have come from another request. This is because the per-request initialization of current_detect_order_list is not unconditional:

static void php_mb_populate_current_detect_order_list(TSRMLS_D)
{
	const mbfl_encoding **entry = 0;
	size_t nentries;

	if (MBSTRG(current_detect_order_list)) {
		return;
	}

If current_detect_order_list is filled with an invalid pointer from a previous request, it will stay so. php_mb_populate_current_detect_order_list() is called from RINIT.

Solution: make php_mb_populate_current_detect_order_list() execute unconditionally.




Expected result:
----------------
no crash

Actual result:
--------------
gdb session (line numbers are for 5.6.31, but it also happens in 5.6.35):

Hardware watchpoint 1: *mbstring_globals.current_detect_order_list
Breakpoint 2 at 0x7f841794defe: file /tmp/php/ext/mbstring/mbstring.c, line 4620.
Breakpoint 3 at 0x7f8421d8350a: file /tmp/php/main/main.c, line 1617.
Breakpoint 4 at 0x7f8421d83c16: file /tmp/php/main/main.c, line 1819.
Breakpoint 5 at 0x7f84179420bb: file /tmp/php/ext/mbstring/mbstring.c, line 1662.
(gdb) c
Continuing.

Breakpoint 3, php_request_startup () at /tmp/php/main/main.c:1617
1617	{
(gdb) 
Continuing.

Breakpoint 2, php_mb_populate_current_detect_order_list () at /tmp/php/ext/mbstring/mbstring.c:4620
4620		const mbfl_encoding **entry = 0;
(gdb) 
Continuing.

Hardware watchpoint 1: *mbstring_globals.current_detect_order_list

Old value = <unreadable>
New value = (const mbfl_encoding *) 0x7f8417c6bb20 <mbfl_encoding_ascii>
php_mb_populate_current_detect_order_list () at /tmp/php/ext/mbstring/mbstring.c:4641
4641		MBSTRG(current_detect_order_list_size) = nentries;
(gdb) 
Continuing.

Breakpoint 4, php_request_shutdown (dummy=0x0) at /tmp/php/main/main.c:1819
1819	{
(gdb) 
Continuing.

Breakpoint 3, php_request_startup () at /tmp/php/main/main.c:1617
1617	{
(gdb)
Continuing.

Breakpoint 2, php_mb_populate_current_detect_order_list () at /tmp/php/ext/mbstring/mbstring.c:4620
4620		const mbfl_encoding **entry = 0;
(gdb)
Continuing.

Hardware watchpoint 1: *mbstring_globals.current_detect_order_list

Old value = (const mbfl_encoding *) 0x7f8417c6bb20 <mbfl_encoding_ascii>
New value = (const mbfl_encoding *) 0x7f8425baebb0
_array_init (arg=0x7f8425baeb38, size=0, __zend_filename=0x7f8421fd7a4e "/tmp/php/main/php_variables.c", __zend_lineno=630) at /tmp/php/Zend/zend_API.c:1013
1013		_zend_hash_init(Z_ARRVAL_P(arg), size, ZVAL_PTR_DTOR, 0 ZEND_FILE_LINE_RELAY_CC);
(gdb)
Continuing.

Breakpoint 4, php_request_shutdown (dummy=0x0) at /tmp/php/main/main.c:1819
1819	{
(gdb)
Continuing.

Hardware watchpoint 1: *mbstring_globals.current_detect_order_list

Old value = (const mbfl_encoding *) 0x7f8425baebb0
New value = (const mbfl_encoding *) 0x5a5a5a5a5a5a5a5a
__memset_avx2 () at ../sysdeps/x86_64/multiarch/memset-avx2.S:86
86	../sysdeps/x86_64/multiarch/memset-avx2.S: No such file or directory.
(gdb)
Continuing.

Breakpoint 3, php_request_startup () at /tmp/php/main/main.c:1617
1617	{
(gdb)
Continuing.

Hardware watchpoint 1: *mbstring_globals.current_detect_order_list

Old value = (const mbfl_encoding *) 0x5a5a5a5a5a5a5a5a
New value = (const mbfl_encoding *) 0x0
__memset_avx2 () at ../sysdeps/x86_64/multiarch/memset-avx2.S:144
144	../sysdeps/x86_64/multiarch/memset-avx2.S: No such file or directory.
(gdb)
Continuing.

Breakpoint 2, php_mb_populate_current_detect_order_list () at /tmp/php/ext/mbstring/mbstring.c:4620
4620		const mbfl_encoding **entry = 0;
(gdb)
Continuing.

Breakpoint 4, php_request_shutdown (dummy=0x0) at /tmp/php/main/main.c:1819
1819	{
(gdb)
Continuing.

Breakpoint 5, zm_deactivate_mbstring (type=1, module_number=24) at /tmp/php/ext/mbstring/mbstring.c:1662
1662	{
(gdb)
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x00007f8421de5a94 in zend_mm_check_ptr (heap=0x55b843b361d0, ptr=0x7f8425baeb38, silent=0, __zend_filename=0x7f8417a50328 "/tmp/php/ext/mbstring/mbstring.c",
    __zend_lineno=1667, __zend_orig_filename=0x0, __zend_orig_lineno=0) at /tmp/php/Zend/zend_alloc.c:1393
1393		    ZEND_MM_PREV_BLOCK(p)->info._size != p->info._prev) {

Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-10-21 11:53 UTC] cmb@php.net
-Status: Open +Status: Verified -Assigned To: +Assigned To: cmb
 [2021-10-21 11:53 UTC] cmb@php.net
The following pull request has been associated:

Patch Name: Fix #76167: mbstring may use pointer from some previous request
On GitHub:  https://github.com/php/php-src/pull/7604
Patch:      https://github.com/php/php-src/pull/7604.patch
 [2021-10-25 10:42 UTC] git@php.net
Automatic comment on behalf of cmb69
Revision: https://github.com/php/php-src/commit/d3d6d7906e0e07efa0e0ca583bae60b0148f037f
Log: Fix #76167: mbstring may use pointer from some previous request
 [2021-10-25 10:42 UTC] git@php.net
-Status: Verified +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Nov 09 13:01:28 2024 UTC