Bug #72610 [Asn]: unserialize() read-after-free when property_table is reallocated

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: 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#L333 https://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) Previous Comments: ------------------------------------------------------------------------ [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