Edit report at https://bugs.php.net/bug.php?id=72610&edit=1
ID: 72610
User updated by: tandre at ifwe dot co
Reported by: tandre at ifwe dot co
Summary: unserialize() read-after-free when property_table is
reallocated
Status: Assigned
Type: Bug
Package: *General Issues
Operating System: All
PHP Version: 7.0.8
Assigned To: dmitry
Block user comment: N
Private report: N
New Comment:
I created a PR to fix this issue: https://github.com/php/php-src/pull/2004 .
Comments and any test cases that you think I should add are welcome.
Also, never mind, https://bugs.php.net/bug.php?id=69295 should be
unaffected by the fix, it won't call __wakeup.
Previous Comments:
------------------------------------------------------------------------
[2016-07-18 02:18:56] tandre at ifwe dot co
It seems like hhvm handles __wakeup in the same way as the second option I mentioned:
https://github.com/facebook/hhvm/blob/2d5f00afbb033aec0cbc51cbe3a897af79cdcb28/hphp/runtime/base/variable-unserializer.cpp#L333https://github.com/facebook/hhvm/blob/2d5f00afbb033aec0cbc51cbe3a897af79cdcb28/hphp/runtime/base/variable-unserializer.cpp#L965
igbinary7 had a similar issue, and a (possibly incomplete) fix for the similar issue was https://github.com/igbinary/igbinary7/pull/17/files
(The NG engine might require slightly different adjustments)
------------------------------------------------------------------------
[2016-07-18 00:43:25] stas@php.net
Doesn't look like security issue, requires a lot of specialized code.
------------------------------------------------------------------------
[2016-07-17 18:12:20] tandre at ifwe dot co
Description:
------------
This affects all versions of phpPHP 7.0.0 to PHP 7.1-alpha3
Running the linked 3v4l test script in php 7 will result in the error "Notice: unserialize():
Error at offset 100 of 102 bytes in /in/1SsOJ on line 28"
Versions from php 7.0.0 to php 7.0.2 are affected slightly differently.
Additionally, Running the test script with the bash command USE_ZEND_ALLOC=0 valgrind php
that_file.php will reveal multiple invalid memory reads of already freed data. See https://pastee.org/tjzp8
I'm not sure if this falls under security, change the bug type if it doesn't. (If new
objects are allocated, I assume they may overlap with the invalid pointers)
Cause:
See ext/standard/var_unserialize.re
The problem is that the var_entries struct contains pointers to zvals in the object
property_table if that property is dynamic (e.g. no declaration in the class such as public
$a).
When the property_table is expanded by realloc(), those pointers usually become invalid.
Possible fixes (not sure if these will work)
- keep a list of copies of those values (instead of pointers) in var_entries (list of
zval instead of zval*) in struct var_entries, and temporarily increment
refcount of underlying objects/arrays?
- Defer calls to __wakeup() until after all properties were set up, and perform those calls in the
same order they originally would have. This may cause different behavior when unserializing.
https://bugs.php.net/bug.php?id=69295 may or may
not be affected by the fix to this bug
Test script:
---------------
https://3v4l.org/DkcB5
Expected result:
----------------
The program runs without reading free()d/realloc()ed memory. It has the below output:
a:2:{i:0;O:3:"Obj":1:{s:1:"a";O:8:"stdClass":1:{s:4:"test";s:3:"foo";}}i:1;O:3:"Obj":1:{s:1:"a";r:3;}}
Called __unserialize
array(2) {
[0]=>
object(Obj)#4 (1) {
["a"]=>
object(stdClass)#5 (1) {
["test"]=>
string(3) "foo"
}
}
[1]=>
object(Obj)#6 (1) {
["a"]=>
object(stdClass)#5 (1) {
["test"]=>
string(3) "foo"
}
}
}
Actual result:
--------------
unserialize performs invalid memory reads, then returns false
a:2:{i:0;O:3:"Obj":1:{s:1:"a";O:8:"stdClass":1:{s:4:"test";s:3:"foo";}}i:1;O:3:"Obj":1:{s:1:"a";r:3;}}
Notice: unserialize(): Error at offset 100 of 102 bytes in /in/DkcB5 on line 20
Called __unserialize
Notice: Trying to get property of non-object in /in/DkcB5 on line 24
Fail 0 b0
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=72610&edit=1