php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #69324 Buffer Over-read in unserialize when parsing Phar
Submitted: 2015-03-29 03:24 UTC Modified: 2015-04-14 07:28 UTC
From: emmanuel dot law at gmail dot com Assigned: stas
Status: Closed Package: PHAR related
PHP Version: 5.6.7 OS: *
Private report: No CVE-ID: 2015-2783
Password:
Status:
Package:
Bug Type:
Summary:
From: emmanuel dot law at gmail dot com
New email:
PHP Version: OS:

 

 [2015-03-29 03:24 UTC] emmanuel dot law at gmail dot com
Description:
------------
The nature of this vulnerability is CWE-126: Buffer Over-read. 
It is possible to read beyond a buffer.

The vulnerability can be triggered when parsing a PHAR file at phar.c:623

		if (!php_var_unserialize(metadata, &p, p + buf_len, &var_hash TSRMLS_CC)) {


"buf_len" is obtained from the phar file and passed into php_var_unserialize() as the max argument.

Under normal php_var_unserialize() circumstances, YYCURSOR will always be <= max. This however can be bypassed when processing a malform phar with a buf_len that is shorter then the string to be unserialized.


It should be noted that YYCURSOR >max should never happen due to lines like var_unserializer.c:893 
	if ((YYLIMIT - YYCURSOR) < 2) YYFILL(2);
However since YYFILL() does nothing, it is optimized away by the compiler and thus never called. This is also a weakness in itself even though the vulnerable point is trigger via phar parsing.

Thus by carefully crafting the string to be unserialize, we can produce a condition where YYCURSOR>max 

When YYCURSOR > max, a buffer over-read conditions occurs and the php_var_unserialize() is in a unstable and vulnerable state. For example var_unserializer.c:906 leads to an integer underflow(or wrap around):
	maxlen = max - YYCURSOR;


I've created a POC that triggers the buffer over-read condition resulting in a memory info leak. This was done by unserializing a "s:<len>:<data>" string object. 

Using other serialized objects might lead to other possible attacks.I'm still in the process of analysing those.

Test script:
---------------
I've created a POC that leaks chunks of memory ala heart-bleed style. 

https://www.dropbox.com/s/tl0o9ekjpsn4s1u/php-buffer-over_read-poc.zip?dl=0

$php POC.php

!!!!!!!!!!!!!!!!!!!! MEM LEAK Found !!!!!!!!!!!!!!!!

     0 : 00 00 00 4d 45 54 41 44 41 54 41 5f 31 32 33 34 [...METADATA_1234]
    10 : 35 36 37 38 39 30 31 32 33 34 35 36 37 38 22 30 [56789012345678"0]
    20 : 31 32 33 22 3b 0b 00 00 00 a5 61 13 55 0b 00 00 [123";.....a.U...]
    30 : 00 82 b7 29 4b b6 01 00 00 00 00 00 00 00 81 01 [...)K...........]
    40 : 00 00 00 00 00 00 79 00 00 00 00 00 00 00 00 24 [......y........$]
.............

  2CA0 : d9 01 01 00 00 00 00 00 01 00 00 00 00 00 49 00 [..............I.]
  2CB0 : 00 00 00 00 00 00 59 00 00 00 00 00 00 00 48 c3 [......Y.......H.]
  2CC0 : 5c 02 01 00 00 00 08 09 0a 0b 0c 0d 0e 0f 10 11 [\...............]
  2CD0 : 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 [.............. !]
####### Mem info leak found(11488 bytes leaked). Continuing to find more...... ######

Actual result:
--------------
Backtrace from parsing the phar file to php_var_unserialize

#0  0x00000000007342db in php_var_unserialize (rval=0x7ffff7fc02a0, p=0x7fffffff9f78, max=0x7ffff7fc001e "77777:\"MY_METADATA_\";\b", var_hash=0x7fffffff9f68)
    at /home/elaw/php-5.6.7/ext/standard/var_unserializer.c:914
#1  0x000000000062ccac in phar_parse_metadata (buffer=0x7fffffffa128, metadata=0x7ffff7fc02a0, zip_metadata_len=0x0) at /home/elaw/php-5.6.7/ext/phar/phar.c:622
#2  0x000000000062ddcf in phar_parse_pharfile (fp=0x7ffff7fc0a38, fname=0x7ffff7fc0f40 "/home/elaw/php-5.6.6-afl-asan/sapi/cli/Phar4-Crash-0.phar", fname_len=0x39, alias=0x0,
    alias_len=0x0, halt_offset=0x1f, pphar=0x7fffffffa868, compression=0x0, error=0x7fffffffa8a8) at /home/elaw/php-5.6.7/ext/phar/phar.c:1038
#3  0x0000000000630c11 in phar_open_from_fp (fp=0x7ffff7fc0a38, fname=0x7ffff7fc0f40 "/home/elaw/php-5.6.6-afl-asan/sapi/cli/Phar4-Crash-0.phar", fname_len=0x39, alias=0x0,
    alias_len=0x0, options=0x8, pphar=0x7fffffffa868, is_data=0x0, error=0x7fffffffa8a8) at /home/elaw/php-5.6.7/ext/phar/phar.c:1716
#4  0x000000000062f8a2 in phar_create_or_parse_filename (fname=0x7ffff7fc0f40 "/home/elaw/php-5.6.6-afl-asan/sapi/cli/Phar4-Crash-0.phar", fname_len=0x39, alias=0x0,
    alias_len=0x0, is_data=0x0, options=0x8, pphar=0x7fffffffa868, error=0x7fffffffa8a8) at /home/elaw/php-5.6.7/ext/phar/phar.c:1346
#5  0x000000000062f7b0 in phar_open_or_create_filename (fname=0x7ffff7fc1c78 "Phar4-Crash-0.phar", fname_len=0x12, alias=0x0, alias_len=0x0, is_data=0x0, options=0x8,
    pphar=0x7fffffffa868, error=0x7fffffffa8a8) at /home/elaw/php-5.6.7/ext/phar/phar.c:1315
#6  0x000000000063b5e6 in zim_Phar___construct (ht=0x2, return_value=0x7ffff7fbe3b0, return_value_ptr=0x7ffff7f874b8, this_ptr=0x7ffff7fbe4a0, return_value_used=0x0)
    at /home/elaw/php-5.6.7/ext/phar/phar_object.c:1189



Patches

phar69324.diff (last revision 2015-04-05 22:11 UTC) by stas@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-03-29 03:34 UTC] emmanuel dot law at gmail dot com
Also note that execution of Phar file is not required. All you need is to open one.
 [2015-04-01 06:54 UTC] emmanuel dot law at gmail dot com
Please use CVE-2015-2783 for this. Thanks
 [2015-04-01 06:59 UTC] kaplan@php.net
-CVE-ID: +CVE-ID: 2015-2783
 [2015-04-05 08:16 UTC] stas@php.net
I'm not sure how cursor can become more than max. YYFILL has nothing to do with it, these checks are not supposed to do anything. Could you provide string which when you pass it to unserialize() makes cursor more than max? 

In your stack trace I see p=0x7fffffff9f78, max=0x7ffff7fc001e which means max is actually less than p, so it looks like it has nothing to do with unserialize. I may be missing something of course so please clarify.
 [2015-04-05 08:53 UTC] stas@php.net
The problem seems to be that while the code in phar_parse_pharfile() checks if the buffer has enough data for lengths, etc. it doesn't seem to check that the buffer has enough data for unserialized data itself. Which results in junk being read as unserialized data.
 [2015-04-05 08:54 UTC] emmanuel dot law at gmail dot com
Hope the following makes it clearer:

1)you CANNOT trigger this via the normal unserial
ize() function in php. It needs to be triggered via phar_parse_metadata () -> php_var_unserialize()

2) When php_var_unserialize(metadata, &p, p + buf_len, &var_hash TSRMLS_CC)() is called  on phar.c:623, max= p +buf_len << in this case buf_len is obtained via a field in the phar file and is thus controllable.


3) As an example to trigger this, make buf_len = 10 and the metadata string to be unserialized be s:000000000008:"whatever" and you will have a buffer-over-read. Remember that this needs to be fed in via a phar file and not the normal unserialize() method.


4) I've a POC exploit that leaks memory info via the buffer over-read here:
https://www.dropbox.com/s/tl0o9ekjpsn4s1u/php-buffer-over_read-poc.zip?dl=0

5)Here's a better stack tract that exemplifies when cursor > max

gdb-peda$ frame
#0  php_var_unserialize (rval=0x7ffff7fc2f50, p=0x7fffffff9f58, max=0x7ffff7fbf1c9 "\"", var_hash=0x7fffffff9f48) at /home/elaw/php-5.6.7/ext/standard/var_unserializer.c:904
904             maxlen = max - YYCURSOR;
gdb-peda$ p max
$4 = (const unsigned char *) 0x7ffff7fbf1c9 "\""
gdb-peda$ p cursor
$5 = (const unsigned char *) 0x7ffff7fbf1ca ""
gdb-peda$ p max-cursor
$6 = 0xffffffffffffffff   <<<< note the integer underflow here
gdb-peda$ bt
#0  php_var_unserialize (rval=0x7ffff7fc2f50, p=0x7fffffff9f58, max=0x7ffff7fbf1c9 "\"", var_hash=0x7fffffff9f48) at /home/elaw/php-5.6.7/ext/standard/var_unserializer.c:904
#1  0x000000000062ccac in phar_parse_metadata (buffer=0x7fffffffa108, metadata=0x7ffff7fc2f50, zip_metadata_len=0x0) at /home/elaw/php-5.6.7/ext/phar/phar.c:622
#2  0x000000000062ddcf in phar_parse_pharfile (fp=0x7ffff7fbdce8, fname=0x7ffff7fbe798 "/home/elaw/php-5.6.6-afl-asan/sapi/cli/newphar_metaonly_hexedited_extended_PHP_POC.phar",
    fname_len=0x57, alias=0x0, alias_len=0x0, halt_offset=0x1c, pphar=0x7fffffffa848, compression=0x0, error=0x7fffffffa888) at /home/elaw/php-5.6.7/ext/phar/phar.c:1038
#3  0x0000000000630c11 in phar_open_from_fp (fp=0x7ffff7fbdce8, fname=0x7ffff7fbe798 "/home/elaw/php-5.6.6-afl-asan/sapi/cli/newphar_metaonly_hexedited_extended_PHP_POC.phar",
    fname_len=0x57, alias=0x0, alias_len=0x0, options=0x8, pphar=0x7fffffffa848, is_data=0x0, error=0x7fffffffa888) at /home/elaw/php-5.6.7/ext/phar/phar.c:1716
#4  0x000000000062f8a2 in phar_create_or_parse_filename (fname=0x7ffff7fbe798 "/home/elaw/php-5.6.6-afl-asan/sapi/cli/newphar_metaonly_hexedited_extended_PHP_POC.phar",
    fname_len=0x57, alias=0x0, alias_len=0x0, is_data=0x0, options=0x8, pphar=0x7fffffffa848, error=0x7fffffffa888) at /home/elaw/php-5.6.7/ext/phar/phar.c:1346
#5  0x000000000062f7b0 in phar_open_or_create_filename (fname=0x7ffff7fbf090 "newphar_metaonly_hexedited_extended_PHP_POC.phar", fname_len=0x30, alias=0x0, alias_len=0x0,
    is_data=0x0, options=0x8, pphar=0x7fffffffa848, error=0x7fffffffa888) at /home/elaw/php-5.6.7/ext/phar/phar.c:1315
#6  0x000000000063b5e6 in zim_Phar___construct (ht=0x2, return_value=0x7ffff7fbc348, return_value_ptr=0x7ffff7f852a0, this_ptr=0x7ffff7fbc438, return_value_used=0x0)
    at /home/elaw/php-5.6.7/ext/phar/phar_object.c:1189
#7  0x000000000084c1b6 in zend_do_fcall_common_helper_SPEC (execute_data=0x7ffff7f85338) at /home/elaw/php-5.6.7/Zend/zend_vm_execute.h:558
#8  0x000000000084c98d in ZEND_DO_FCALL_BY_NAME_SPEC_HANDLER (execute_data=0x7ffff7f85338) at /home/elaw/php-5.6.7/Zend/zend_vm_execute.h:693
#9  0x000000000084b81f in execute_ex (execute_data=0x7ffff7f85338) at /home/elaw/php-5.6.7/Zend/zend_vm_execute.h:363
#10 0x000000000084b8a8 in zend_execute (op_array=0x7ffff7fbd248) at /home/elaw/php-5.6.7/Zend/zend_vm_execute.h:388
#11 0x0000000000808067 in zend_execute_scripts (type=0x8, retval=0x0, file_count=0x3) at /home/elaw/php-5.6.7/Zend/zend.c:1341
#12 0x0000000000773643 in php_execute_script (primary_file=0x7fffffffdf60) at /home/elaw/php-5.6.7/main/main.c:2597
#13 0x00000000008b9643 in do_cli (argc=0x3, argv=0xf92f10) at /home/elaw/php-5.6.7/sapi/cli/php_cli.c:994
#14 0x00000000008ba750 in main (argc=0x3, argv=0xf92f10) at /home/elaw/php-5.6.7/sapi/cli/php_cli.c:1378
#15 0x00007ffff624eb45 in __libc_start_main (main=0x8ba0b0 <main>, argc=0x3, argv=0x7fffffffe2e8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>,
    stack_end=0x7fffffffe2d8) at libc-start.c:287
#16 0x00000000004216d9 in _start ()
 [2015-04-05 09:07 UTC] emmanuel dot law at gmail dot com
"it doesn't seem to check that the buffer has enough data for unserialized data itselfWhich results in junk being read as unserialized data." 

<< Exactly! and this is exploitable.

Note that YYFILL() would have prevented something like this from being exploitable. But it currently is just an empty loop and is optimised away by the compiler. Maybe you want to consider fixing this too as I think it is a weakness that could potentially be exploited by future vulnerabilities like this.
 [2015-04-05 20:54 UTC] stas@php.net
Again, YYFILL is not supposed to do anything. It's supposed to be empty loop. YYFILL is needed when we're reading from stream, etc. but we are not. See http://re2c.org/manual.html. In the case of unserializing, buffer does not need filling - it's already full. Of course, putting max beyond the end of the actual data is a problem - but it's not unserialize problem, it's phar problem.
 [2015-04-05 20:55 UTC] stas@php.net
The following patch has been added/updated:

Patch Name: phar69324.diff
Revision:   1428267328
URL:        https://bugs.php.net/patch-display.php?bug=69324&patch=phar69324.diff&revision=1428267328
 [2015-04-05 20:56 UTC] stas@php.net
Please try this patch, it should fix the issue: https://gist.github.com/smalyshev/f362a4cb87c01400df00
 [2015-04-05 22:11 UTC] stas@php.net
The following patch has been added/updated:

Patch Name: phar69324.diff
Revision:   1428271874
URL:        https://bugs.php.net/patch-display.php?bug=69324&patch=phar69324.diff&revision=1428271874
 [2015-04-06 05:20 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2015-04-06 11:14 UTC] emmanuel dot law at gmail dot com
I dont have access to  https://bugs.php.net/patch-display.php?bug=69324&patch=phar69324.diff&revision=1428271874

But https://gist.github.com/smalyshev/f362a4cb87c01400df00 does not solve the problem. If you want to share "Revision:1428271874" with me, I could take a look.

IMHO, the patch seems to be overly complicated.  Here's my 2 cents of worth of pseudo code. 


1) Instead of passing the phar buffer( p in this case) directly to php_var_unserialize(), we would allocate a new buffer containing the metadata string to be unserialised.


2) Read in Metadata len from phar file. Ensure that len is smaller the (endbuffer - current buffer). This seems to be what you have already done.

3)Create a new buffer for the metadata string to be unserialized. EG: MetaData_to_beunserialized = estrndup(buffer, len)  <<<estrndup is important because it adds a nullbyte to the end of the string. This is really the crux of the fix. It makes it impossible to pass in a malform string that unserialized properly when YYCURSOR > YYMAX

3) Call your the unserialized function as per normal but use the new buffer instead. EG:
php_var_unserialize(metadata, &MetaData_to_beunserialized, p + buf_len, &var_hash TSRMLS_CC)


It's mid-night here.. so I hope I'm making sense?
 [2015-04-06 15:26 UTC] stas@php.net
Could you please explain what do you mean by "does not solve the problem"? I can not reproduce the problem with either your code or my tests anymore with this patch, but if you have some other reproduction, please provide it. If it's the same reproduction, could you please share on which system you reproduced it, as it does not reproduce on any system I have access to.

As for copying the buffers around, I don't think it is a good solution. I don't see how it adds anything to the solution and it is expensive performance-wise.
 [2015-04-06 15:41 UTC] stas@php.net
After looking at the code mode I think I understand what additional problem may be - unserialize does not check the max boundary when inside parsing the number. So if the number string is long it can overrun the max. Usually the php strings end in \0 so it would stop there but in this case since it's a phar buffer it isn't null-terminated. I'll see what is the best solution for the problem.
 [2015-04-07 01:54 UTC] emmanuel dot law at gmail dot com
Precisely. I think we are on the same page now. Apologies if I didn't explain it clearly enough.
 [2015-04-14 07:29 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=9faaee66fa493372c7340b1ab05f8fd115131a42
Log: Fixed bug #69324 (Buffer Over-read in unserialize when parsing Phar)
 [2015-04-14 07:29 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2015-04-14 08:31 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=9faaee66fa493372c7340b1ab05f8fd115131a42
Log: Fixed bug #69324 (Buffer Over-read in unserialize when parsing Phar)
 [2015-04-15 08:43 UTC] jpauli@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=17cbd0b5b78a7500f185b3781a2149881bfff8ae
Log: Fixed bug #69324 (Buffer Over-read in unserialize when parsing Phar)
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Mon Aug 21 16:01:42 2017 UTC