php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #70433 Uninitialized pointer in phar_make_dirstream when zip entry filename is "/"
Submitted: 2015-09-05 11:42 UTC Modified: 2015-10-11 10:53 UTC
From: hugh at allthethings dot co dot nz Assigned: kaplan
Status: Closed Package: PHAR related
PHP Version: 5.6Git-2015-09-05 (Git) OS: Linux
Private report: No CVE-ID: 2015-7804
 [2015-09-05 11:42 UTC] hugh at allthethings dot co dot nz
Description:
------------
When parsing a phar file in zip format (phar_parse_zipfile in ext/phar/zip.c) with a directory entry with filename /, the filename length is 1 to start with, but then because it is a directory, it is decremented to 0 (line 399 of ext/phar/zip.c), which causes it to get added to the manifest hash as a type LONG not as a type STRING (line 639 of ext/phar/zip.c).
When making the dirstream from the phar file after loading the zip (function phar_make_dirstream in ext/phar/dirstream.c), the manifest hash is used, but the return is not checked for a STRING type, which is what it expects (line 201 in ext/phar/dirstream.c). In this case, a LONG is returned, and the two variables str_key and keylen are not set. The str_key is further used to set the filename.

This will cause a crash when compiled with hardening options such as -D_FORTIFY_SOURCE=2 and -fstack-protector-all AND NO optimizations (-O flags), OR when compiled with ASAN. When compiled any other way that I've tried, it doesn't crash, but occasionally leaks data into the filename field outputted.

This bug was found with afl-fuzz.

The zip file passed to the script below is available at https://transfer.sh/VULId/fuzz-test.zip and has the hexdump of below:

00000000  50 4b 03 04 30 30 30 30  30 30 30 30 30 30 30 30  |PK..000000000000|
00000010  30 30 30 30 30 30 30 30  30 30 30 30 30 30 30 30  |0000000000000000|
*
000000c0  30 30 30 30 30 30 30 30  50 4b 01 02 30 30 30 30  |00000000PK..0000|
000000d0  30 30 08 00 30 30 30 30  30 30 30 30 30 30 30 30  |00..000000000000|
000000e0  30 30 30 30 01 00 00 00  00 00 30 30 30 30 30 30  |0000......000000|
000000f0  30 30 30 30 30 30 2f 50  4b 05 06 00 00 00 00 01  |000000/PK.......|
00000100  00 01 00 30 30 30 30 c8                           |...0000.|
00000108


To fix:

a) check the return type of zend_hash_get_current_key_ex for more than just non existent (ie, for LONG as well), maybe fail if type is not STRING?

or b) don't decrement the entry.filename_len in parse_zip if it would set the length to 0, that would mean it is stored in the hash as a string, not a long. I'm not too sure of the logic of this, so don't know if that reasoning is right.

Also, I haven't checked other filetypes for phar, potentially a similar issue is  there as well!

Cheers,

Hugh

Test script:
---------------
<?php
$phar = new PharData($argv[1]);
var_dump($phar);
$meta = $phar->getMetadata();
var_dump($meta);
?>

Expected result:
----------------
No crash or memory leaked.

Actual result:
--------------
When compiled with ASAN

(gdb) bt
#0  0x00007ffff4e5a651 in memcmp () from /usr/lib/x86_64-linux-gnu/libasan.so.0
#1  0x0000000001338f6c in phar_make_dirstream (dir=0x7ffff7f75ac8 "/", manifest=0x7ffff7f756f0) at /root/php-src/ext/phar/dirstream.c:216
#2  0x000000000133abb9 in phar_wrapper_open_dir (wrapper=<optimized out>, path=<optimized out>, mode=<optimized out>, options=0, opened_path=<optimized out>, 
    context=0x0) at /root/php-src/ext/phar/dirstream.c:359
#3  0x0000000001a0a26d in _php_stream_opendir (path=path@entry=0x7ffff7f759c8 "phar:///root/fuzz-test.zip/", options=options@entry=8, context=0x0)
    at /root/php-src/main/streams/streams.c:1986
#4  0x000000000155528e in spl_filesystem_dir_open (intern=intern@entry=0x7ffff7f76f40, path=0x7ffff7f759c8 "phar:///root/fuzz-test.zip/")
    at /root/php-src/ext/spl/spl_directory.c:250
#5  0x00000000015642d2 in spl_filesystem_object_construct (ctor_flags=1, return_value_used=<optimized out>, this_ptr=0x7ffff7f74808, return_value_ptr=<optimized out>, 
    return_value=<optimized out>, ht=<optimized out>) at /root/php-src/ext/spl/spl_directory.c:731
#6  zim_spl_RecursiveDirectoryIterator___construct (ht=<optimized out>, return_value=<optimized out>, return_value_ptr=<optimized out>, this_ptr=0x7ffff7f74808, 
    return_value_used=<optimized out>) at /root/php-src/ext/spl/spl_directory.c:1596
#7  0x0000000001c0bdf8 in zend_call_function (fci=fci@entry=0x7fffffff9fd0, fci_cache=0x7fffffff9f70) at /root/php-src/Zend/zend_execute_API.c:847
#8  0x0000000001d4596a in zend_call_method (object_pp=object_pp@entry=0x7fffffffa450, obj_ce=<optimized out>, fn_proxy=fn_proxy@entry=0x60360001f360, 
    function_name=function_name@entry=0x27a36c0 "__construct", function_name_len=function_name_len@entry=11, retval_ptr_ptr=retval_ptr_ptr@entry=0x0, 
    param_count=param_count@entry=2, arg1=arg1@entry=0x7fffffffa490, arg2=arg2@entry=0x7fffffffa4d0) at /root/php-src/Zend/zend_interfaces.c:97
#9  0x0000000001376f7e in zim_Phar___construct (ht=<optimized out>, return_value=<optimized out>, return_value_ptr=<optimized out>, this_ptr=<optimized out>, 
    return_value_used=<optimized out>) at /root/php-src/ext/phar/phar_object.c:1255
#10 0x00000000022b4886 in zend_do_fcall_common_helper_SPEC (execute_data=0x7ffff7f419e0) at /root/php-src/Zend/zend_vm_execute.h:558
#11 0x0000000001e6bd72 in execute_ex (execute_data=0x7ffff7f419e0) at /root/php-src/Zend/zend_vm_execute.h:363
#12 0x0000000001c9602f in zend_execute_scripts (type=type@entry=8, retval=retval@entry=0x0, file_count=file_count@entry=3) at /root/php-src/Zend/zend.c:1341
#13 0x0000000001956e42 in php_execute_script (primary_file=primary_file@entry=0x7fffffffd290) at /root/php-src/main/main.c:2597
#14 0x00000000022c0715 in do_cli (argc=3, argv=0x60060000ec80) at /root/php-src/sapi/cli/php_cli.c:994
#15 0x000000000046d184 in main (argc=3, argv=0x60060000ec80) at /root/php-src/sapi/cli/php_cli.c:1378
(gdb) frame 1
#1  0x0000000001338f6c in phar_make_dirstream (dir=0x7ffff7f75ac8 "/", manifest=0x7ffff7f756f0) at /root/php-src/ext/phar/dirstream.c:216
216                             if (keylen >= sizeof(".phar")-1 && !memcmp(str_key, ".phar", sizeof(".phar")-1)) {
(gdb) print str_key
$1 = 0x0
(gdb) print keylen
$2 = 28369926                                                                      

When compiled without optimization and with hardening options
(gdb) bt
#0  __memcmp_sse4_1 () at ../sysdeps/x86_64/multiarch/memcmp-sse4.S:878
#1  0x00000000007a79b2 in phar_make_dirstream (dir=0x7ffff7fe1e48 "/", 
    manifest=0x7ffff7fe1a70) at /root/php-src/ext/phar/dirstream.c:216
#2  0x00000000007a8c60 in phar_wrapper_open_dir (
    wrapper=0x14943e0 <php_stream_phar_wrapper>, 
    path=0x7ffff7fe1d48 "phar:///root/fuzz-test.zip/", mode=0x122529d "r", 
    options=0, opened_path=0x0, context=0x0)
    at /root/php-src/ext/phar/dirstream.c:367
#3  0x0000000000b5ad9b in _php_stream_opendir (
    path=0x7ffff7fe1d48 "phar:///root/fuzz-test.zip/", options=8, context=0x0)
    at /root/php-src/main/streams/streams.c:1986
#4  0x0000000000893374 in spl_filesystem_dir_open (intern=0x7ffff7fe32c0, 
    path=0x7ffff7fe1d48 "phar:///root/fuzz-test.zip/")
    at /root/php-src/ext/spl/spl_directory.c:250
#5  0x0000000000897b17 in spl_filesystem_object_construct (ht=2, 
    return_value=0x7ffff7fe1da8, return_value_ptr=0x7fffffffaa40, 
    this_ptr=0x7ffff7fe0bd8, return_value_used=1, ctor_flags=1)
    at /root/php-src/ext/spl/spl_directory.c:731
#6  0x000000000089ff8c in zim_spl_RecursiveDirectoryIterator___construct (ht=2, 
    return_value=0x7ffff7fe1da8, return_value_ptr=0x7fffffffaa40, 
    this_ptr=0x7ffff7fe0bd8, return_value_used=1)
    at /root/php-src/ext/spl/spl_directory.c:1596
#7  0x0000000000ca83c5 in zend_call_function (fci=0x7fffffffaab0, 
    fci_cache=0x7fffffffaa80) at /root/php-src/Zend/zend_execute_API.c:847
#8  0x0000000000d69dd1 in zend_call_method (object_pp=0x7fffffffabe8, 
    obj_ce=0x15af210, fn_proxy=0x154cad0, function_name=0x119a752 "__construct", 
    function_name_len=11, retval_ptr_ptr=0x0, param_count=2, arg1=0x7fffffffac10, 
    arg2=0x7fffffffac30) at /root/php-src/Zend/zend_interfaces.c:97
#9  0x00000000007dc5f5 in zim_Phar___construct (ht=1, return_value=0x7ffff7fe0b78, 
    return_value_ptr=0x7ffff7fae178, this_ptr=0x7ffff7fe0bd8, return_value_used=0)
    at /root/php-src/ext/phar/phar_object.c:1255
#10 0x0000000000dee160 in zend_do_fcall_common_helper_SPEC (
    execute_data=0x7ffff7fae1f0) at /root/php-src/Zend/zend_vm_execute.h:558
#11 0x0000000000df09c9 in ZEND_DO_FCALL_BY_NAME_SPEC_HANDLER (
    execute_data=0x7ffff7fae1f0) at /root/php-src/Zend/zend_vm_execute.h:693
#12 0x0000000000de9a05 in execute_ex (execute_data=0x7ffff7fae1f0)
    at /root/php-src/Zend/zend_vm_execute.h:363
#13 0x0000000000deae41 in zend_execute (op_array=0x7ffff7fe13f8)
    at /root/php-src/Zend/zend_vm_execute.h:388
#14 0x0000000000cf8d99 in zend_execute_scripts (type=8, retval=0x0, file_count=3)
    at /root/php-src/Zend/zend.c:1341
#15 0x0000000000ae8697 in php_execute_script (primary_file=0x7fffffffd5b0)
    at /root/php-src/main/main.c:2597
#16 0x00000000010799cf in do_cli (argc=3, argv=0x14b67a0)
    at /root/php-src/sapi/cli/php_cli.c:994
#17 0x000000000107c553 in main (argc=3, argv=0x14b67a0)
    at /root/php-src/sapi/cli/php_cli.c:1378
(gdb) frame 1
#1  0x00000000007a79b2 in phar_make_dirstream (dir=0x7ffff7fe1e48 "/", 
    manifest=0x7ffff7fe1a70) at /root/php-src/ext/phar/dirstream.c:226
216                             if (keylen >= sizeof(".phar")-1 && !memcmp(str_key, ".phar", sizeof(".phar")-1)) {
(gdb) p str_key
$1 = 0xcc164cd7fef0fc00 <error: Cannot access memory at address 0xcc164cd7fef0fc00>
(gdb) p keylen
$2 = 19

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-09-05 11:50 UTC] hugh at allthethings dot co dot nz
Forgot to give some output of bad memory returned to userland:

07:48 $ ./php-no-hard-no-opt ~/fuzzing/php/phar.php fuzz-test.zip
object(PharData)#1 (4) {
  ["pathName":"SplFileInfo":private]=>
  string(37) "phar:///root/php-src/fuzz-test.zip/¿"
  ["fileName":"SplFileInfo":private]=>
  string(2) "¿"
  ["glob":"DirectoryIterator":private]=>
  bool(false)
  ["subPathName":"RecursiveDirectoryIterator":private]=>
  string(0) ""
}
NULL

07:49 $ ./php-no-hard-no-opt ~/fuzzing/php/phar.php fuzz-test.zip
object(PharData)#1 (4) {
  ["pathName":"SplFileInfo":private]=>
  string(37) "phar:///root/php-src/fuzz-test.zip/Pj"
  ["fileName":"SplFileInfo":private]=>
  string(2) "Pj"
  ["glob":"DirectoryIterator":private]=>
  bool(false)
  ["subPathName":"RecursiveDirectoryIterator":private]=>
  string(0) ""
}
NULL
 [2015-09-05 11:55 UTC] hugh at allthethings dot co dot nz
Looks like ext/phar/tar.c does the decrement on line 420 or 440 then stored on line 493, so that will have the same issue.
 [2015-09-09 03:56 UTC] hugh at allthethings dot co dot nz
Hey,

Just checking that someone has seen this?

Cheers,

Hugh
 [2015-09-22 07:55 UTC] hugh at allthethings dot co dot nz
Hey,

Just checking that someone has seen this?

Cheers,

Hugh
 [2015-09-28 23:18 UTC] stas@php.net
The URL https://transfer.sh/VULId/fuzz-test.zip doesn't seem to work
 [2015-09-29 03:46 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=e78ac461dbefb7c4a3e9fde78d50fbc56b7b0183
Log: FIx bug #70433 - Uninitialized pointer in phar_make_dirstream when zip entry filename is &quot;/&quot;
 [2015-09-29 03:46 UTC] stas@php.net
-Status: Open +Status: Closed
 [2015-09-29 13:10 UTC] ab@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=e78ac461dbefb7c4a3e9fde78d50fbc56b7b0183
Log: FIx bug #70433 - Uninitialized pointer in phar_make_dirstream when zip entry filename is &quot;/&quot;
 [2015-10-11 10:53 UTC] kaplan@php.net
-Assigned To: +Assigned To: kaplan -CVE-ID: +CVE-ID: 2015-7804
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Apr 30 01:01:34 2017 UTC