php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: cyoung at tripwire dot com
New email:
PHP Version: OS:

 

 [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

Add a Patch

Pull Requests

Add a Pull Request

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
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Mar 29 06:01:29 2024 UTC