Also note that execution of Phar file is not required. All you need is to open one.

Please use CVE-2015-2783 for this. Thanks

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.

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.

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

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

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.

Please try this patch, it should fix the issue: https://gist.github.com/smalyshev/f362a4cb87c01400df00

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?

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.

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.

Precisely. I think we are on the same page now. Apologies if I didn't explain it clearly enough.