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 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

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
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Dec 03 17:01:29 2024 UTC