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

Edit report at https://bugs.php.net/bug.php?id=72610&edit=1 ID: 72610 Updated by: stas@php.net Reported by: tandre at ifwe dot co Summary: unserialize() read-after-free when property_table is reallocated Status: Open -Type: Security +Type: Bug Package: *General Issues Operating System: All PHP Version: 7.0.8 -Assigned To: +Assigned To: dmitry Block user comment: N Private report: Y New Comment: Doesn't look like security issue, requires a lot of specialized code. Previous Comments: ------------------------------------------------------------------------ [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