Bug #69227 [Ver->Csd]: Use after free in zval_scan caused by spl_object_storage_get_gc

Edit report at https://bugs.php.net/bug.php?id=69227&edit=1 ID: 69227 Updated by: laruence@php.net Reported by: adam at vektah dot net Summary: Use after free in zval_scan caused by spl_object_storage_get_gc -Status: Verified +Status: Closed Type: Bug Package: SPL related Operating System: ubuntu 14.10 PHP Version: 5.5.22 Assigned To: laruence Block user comment: N Private report: N New Comment: Automatic comment on behalf of adam.scarr@99designs.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=950d3d6e9b94b75b266c67bf9e3a85ae9c31905d Log: Fix bug #69227 and #65967 Previous Comments: ------------------------------------------------------------------------ [2015-03-13 04:10:30] adam at vektah dot net What about using the the *zval[] table, instead of modifying properties eg: https://github.com/php/php-src/pull/1175 ------------------------------------------------------------------------ [2015-03-12 10:38:27] adam at vektah dot net Wouldn't the refcount always be zero during the sweep? mark has already decremented everything by 1. Does it need to be in props at all? It looks like it was tweaked back in 2012 from an implementation that needed to modify props: https://github.com/php/php-src/commit/df97c3aa0d331be668bd5d8f27fff96d4e3ac1d7 It also seems to be causing other issues eg: https://bugs.php.net/bug.php?id=65967 ------------------------------------------------------------------------ [2015-03-12 10:31:55] laruence@php.net s ,going to be freed ,processing, ------------------------------------------------------------------------ [2015-03-12 10:18:40] laruence@php.net A quick fix could be: diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c index 5e21088..4fc0b6f 100644 --- a/ext/spl/spl_observer.c +++ b/ext/spl/spl_observer.c @@ -378,6 +378,10 @@ static HashTable *spl_object_storage_get_gc(zval *obj, zval ***table, int *n TSR /* clean \x00gcdata, as it may be out of date */ if (zend_hash_find(props, "\x00gcdata", sizeof("\x00gcdata"), (void**) &gcdata_arr_pp) == SUCCESS) { + if (Z_REFCOUNT_PP(gcdata_arr_pp) == 0) { + /* this is going to be freed by gc, don't make a new one */ + return props; + } gcdata_arr = *gcdata_arr_pp; zend_hash_clean(Z_ARRVAL_P(gcdata_arr)); } but let me think of side affects... ------------------------------------------------------------------------ [2015-03-12 09:04:34] laruence@php.net confirm, relates to recursively calls to get_gc. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=69227 -- Edit this bug report at https://bugs.php.net/bug.php?id=69227&edit=1