Skip to content

Fixed #74021 - fetch_array returned broken data#2357

Closed
andrewnester wants to merge 1 commit intophp:PHP-7.1from
andrewnester:74021-fetch-array
Closed

Fixed #74021 - fetch_array returned broken data#2357
andrewnester wants to merge 1 commit intophp:PHP-7.1from
andrewnester:74021-fetch-array

Conversation

@andrewnester
Copy link
Contributor

@andrewnester andrewnester commented Feb 1, 2017

@krakjoe krakjoe added the Bug label Feb 1, 2017
@nikic
Copy link
Member

nikic commented Feb 1, 2017

I don't think the linked commit fixes the issue from #2249. It only concerns bit fields.

@andrewnester
Copy link
Contributor Author

andrewnester commented Feb 1, 2017

@nikic I've added bug73800.phpt to check that #2249 doesn't occur.

@nikic nikic self-requested a review February 1, 2017 18:29
@nikic nikic self-assigned this Feb 12, 2017
@nikic
Copy link
Member

nikic commented Feb 12, 2017

As suspected, when running the test for bug 73800 you added under valgrind, I get memory errors:

==26369== Invalid read of size 1
==26369==    at 0xABBE49: php_mysqlnd_rowp_read_text_protocol_aux (mysqlnd_wireprotocol.c:1703)
==26369==    by 0xABC57F: php_mysqlnd_rowp_read_text_protocol_zval (mysqlnd_wireprotocol.c:1787)
==26369==    by 0xAF0927: mysqlnd_mysqlnd_result_buffered_zval_fetch_row_pub (mysqlnd_result.c:1096)
==26369==    by 0xAF2146: mysqlnd_mysqlnd_res_fetch_row_pub (mysqlnd_result.c:1275)
==26369==    by 0xAF6BFD: mysqlnd_mysqlnd_res_fetch_into_pub (mysqlnd_result.c:1754)
==26369==    by 0x7C80A6: php_mysqli_fetch_into_hash_aux (mysqli.c:1220)
==26369==    by 0x7C843E: php_mysqli_fetch_into_hash (mysqli.c:1271)
==26369==    by 0x7D836B: zif_mysqli_fetch_array (mysqli_nonapi.c:350)
==26369==    by 0xC76D40: ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (zend_vm_execute.h:1079)
==26369==    by 0xC74825: execute_ex (zend_vm_execute.h:432)
==26369==    by 0xC74AC4: zend_execute (zend_vm_execute.h:474)
==26369==    by 0xC05E7A: zend_execute_scripts (zend.c:1544)
==26369==  Address 0x11d9d8dc is 0 bytes after a block of size 18,000,028 alloc'd
==26369==    at 0x4C2FD5F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26369==    by 0xBC29E5: __zend_realloc (zend_alloc.c:2836)
==26369==    by 0xBC1A1F: _erealloc (zend_alloc.c:2443)
==26369==    by 0xAA2A22: _mysqlnd_erealloc (mysqlnd_alloc.c:267)
==26369==    by 0xB10147: mysqlnd_mempool_resize_chunk (mysqlnd_block_alloc.c:94)
==26369==    by 0xAB9B4A: php_mysqlnd_read_row_ex (mysqlnd_wireprotocol.c:1511)
==26369==    by 0xABCCFD: php_mysqlnd_rowp_read (mysqlnd_wireprotocol.c:1826)
==26369==    by 0xAF26A4: mysqlnd_mysqlnd_res_store_result_fetch_data_pub (mysqlnd_result.c:1326)
==26369==    by 0xAF373F: mysqlnd_mysqlnd_res_store_result_pub (mysqlnd_result.c:1443)
==26369==    by 0xA98876: mysqlnd_mysqlnd_conn_data_store_result_pub (mysqlnd_connection.c:1965)
==26369==    by 0x7D98B1: zif_mysqli_query (mysqli_nonapi.c:617)
==26369==    by 0xC76D40: ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (zend_vm_execute.h:1079)

@nikic
Copy link
Member

nikic commented Feb 12, 2017

The problem with the original patch appears to be that the realloc case is not handled correctly -- we pretend like the extra byte is part of the data there.

@nikic
Copy link
Member

nikic commented Feb 12, 2017

Merged via 01c1afa with a different fix. Now leaving data_size alone and instead only adding the extra byte when performing the actual alloc/realloc.

@nikic nikic closed this Feb 12, 2017
@nikic nikic removed their request for review February 12, 2017 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nikic nikic

Labels

Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@andrewnester @nikic @krakjoe