Bug 949962 (CVE-2015-7804) - VUL-1: CVE-2015-7804: php5,php53: Uninitialized pointer in phar_make_dirstream when zip entry filename is "/"
Summary: VUL-1: CVE-2015-7804: php5,php53: Uninitialized pointer in phar_make_dirstrea...
Status: RESOLVED FIXED
Alias: CVE-2015-7804
Product: SUSE Security Incidents
Classification: Novell Products
Component: Incidents (show other bugs)
Version: unspecified
Hardware: Other openSUSE 13.2
: P4 - Low : Normal
Target Milestone: ---
Assignee: Security Team bot
QA Contact: Security Team bot
URL: https://smash.suse.de/issue/157567/
Whiteboard: CVSSv2:RedHat:CVE-2015-7804:4.3:(AV:N...
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-12 09:54 UTC by Andreas Stieger
Modified: 2016-08-09 18:23 UTC (History)
1 user (show)

See Also:
Found By: Security Response Team
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
testcase zip (264 bytes, application/zip)
2015-10-14 13:50 UTC, Petr Gajdos
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Stieger 2015-10-12 09:54:33 UTC
https://bugs.php.net/bug.php?id=70433

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



References:
http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2015-7804
http://seclists.org/oss-sec/2015/q4/58
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-7804
Comment 1 Andreas Stieger 2015-10-12 12:36:33 UTC
commit e78ac461dbefb7c4a3e9fde78d50fbc56b7b0183
Author: Stanislav Malyshev <stas@php.net>
Date:   Mon Sep 28 17:12:35 2015 -0700

    FIx bug #70433 - Uninitialized pointer in phar_make_dirstream when zip entry filename is "/"

commit 1ddf72180a52d247db88ea42a3e35f824a8fbda1
Author: Stanislav Malyshev <stas@php.net>
Date:   Mon Sep 28 21:37:26 2015 -0700

    Better fix for bug #70433
Comment 3 Swamp Workflow Management 2015-10-12 22:00:55 UTC
bugbot adjusting priority
Comment 4 Petr Gajdos 2015-10-14 13:50:55 UTC
Created attachment 651517 [details]
testcase zip

Tested with 13.2:

$ cat bug70433.php
<?php
$phar = new PharData(__DIR__."/bug70433.zip");
var_dump($phar);
$meta = $phar->getMetadata();
var_dump($meta);
?>

$ gdb php
(gdb) b dirstream.c:201
(gdb) run bug70433.php
Breakpoint 1, phar_make_dirstream (dir=0x7ffff7fdcfa8 "/", manifest=0x7ffff7fdcbd0) at /usr/src/debug/php-5.6.1/ext/phar/dirstream.c:201
201			if (HASH_KEY_NON_EXISTENT == zend_hash_get_current_key_ex(manifest, &str_key, &keylen, &unused, 0, NULL)) {
(gdb) n
205			if (keylen <= (uint)dirlen) {


BEFORE

(gdb) p str_key
$1 = 0xf4d6d34db9e7fd00 <error: Cannot access memory at address 0xf4d6d34db9e7fd00>
(gdb)

(segfaults)

AFTER
(gdb) p str_key
$1 = 0x7ffff7fdcd50 "/\272\272\272\272\272\272\272\271"
(gdb) p keylen
$2 = 1
Comment 5 Petr Gajdos 2015-10-14 14:09:04 UTC
I will try to explain why I don't think 5.5.14 and less are affected.

The important part is, that _zend_hash_add_or_update() does not allow empty key:

        if (nKeyLength <= 0) {
                return FAILURE;
        }

in opposite to 5.6.1 we have in 13.2 (there is ZEND_ASSERT() call, but it is active only when ZEND_DEBUG).

The difference happens on zip.c:639. For 5.6.1, we have

639			zend_hash_add(&mydata->manifest, entry.filename, entry.filename_len, (void *)&entry,sizeof(phar_entry_info), NULL);
(gdb) n
(gdb) p mydata->manifest->nNumOfElements
$20 = 1

and for 5.5.14 on the other side

(gdb) p mydata->manifest->nNumOfElements
$5 = 0

Later on, for 5.6.1, in dirstream.c, line 191:

191		if ((*dir == '/' && dirlen == 1 && (manifest->nNumOfElements == 0)) || (dirlen >= sizeof(".phar")-1 && !memcmp(dir, ".phar", sizeof(".phar")-1))) {
(gdb) n
198		zend_hash_internal_pointer_reset(manifest);

but for 5.5.14:

200		if ((*dir == '/' && dirlen == 1 && (manifest->nNumOfElements == 0)) || (dirlen >= sizeof(".phar")-1 && !memcmp(dir, ".phar", sizeof(".phar")-1))) {
(gdb) n
203			efree(dir);

So, for 5.5.14, it bails out before the offending code:

        while (FAILURE != zend_hash_has_more_elements(manifest)) {
                if (HASH_KEY_NON_EXISTENT == zend_hash_get_current_key_ex(manifest, &str_key, &keylen, &unused, 0, NULL)) {
                        break;
                }


because manifest->nNumOfElements is equal to zero, in opposite to 5.6.1.
Comment 6 Petr Gajdos 2016-01-18 12:41:18 UTC
Submitted for 13.2. As per comment 5, other versions are not currently affected.
Comment 7 Bernhard Wiedemann 2016-01-18 13:00:24 UTC
This is an autogenerated message for OBS integration:
This bug (949962) was mentioned in
https://build.opensuse.org/request/show/354582 13.2 / php5
Comment 8 Andreas Stieger 2016-01-26 14:14:25 UTC
Releasing 13.2 update
Comment 9 Swamp Workflow Management 2016-01-26 17:15:24 UTC
openSUSE-SU-2016:0251-1: An update that fixes three vulnerabilities is now available.

Category: security (moderate)
Bug References: 949961,949962,962057
CVE References: CVE-2015-7803,CVE-2015-7804,CVE-2016-1903
Sources used:
openSUSE 13.2 (src):    php5-5.6.1-39.1