Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash in zend_objects_store_free_object_storage due to uninitialised handlers #11734

Closed
tstarling opened this issue Jul 18, 2023 · 2 comments
Closed

Comments

@tstarling
Copy link
Contributor

tstarling commented Jul 18, 2023

Description

<?php

ini_set('memory_limit', '2M');
$objs = [];
while (true) {
        $objs[] = new SplPriorityQueue;
}

This crashes on shutdown:

Program received signal SIGSEGV, Segmentation fault.
0x00005555558e4187 in zend_objects_store_free_object_storage (objects=objects@entry=0x555555ad71a8 <executor_globals+840>, fast_shutdown=fast_shutdown@entry=true) at ./Zend/zend_objects_API.c:107
...
(gdb) print obj->handlers
$2 = (const zend_object_handlers *) 0x0

When an object is created, there is a critical section between zend_object_std_init() and the assignment to handlers during which time it is unsafe to call the Zend allocator.

This was reported in LuaSandbox (T322748) but a quick review suggests that many other extensions have the same issue. In the PHP 8.1 tree, there is:

  • dom_nodemap_or_nodelist_objects_new
  • pdo_dbh_new
  • spl_dllist_object_new_ex
  • spl_heap_object_new_ex
  • spl_object_storage_new_ex
  • spl_array_object_new_ex
  • tidy_object_new
  • xsl_objects_new

My test case above uses the bug in spl_heap_object_new_ex().

Potential rectification options:

  1. In zend_objects_store_free_object_storage, guard against !obj->handlers. I think this is a good solution since it should fix the issue in all extensions. obj->handlers is set to NULL in zend_object_alloc(). There are not many ways to access an object when there was a fatal error before a reference was taken. Maybe a guard in zend_objects_store_call_destructors() would also be wise.
  2. Fix all extensions. Update the comment on zend_object_alloc() to indicate that, in addition to the other requirements, handlers must be set before any allocation is done.

PHP Version

PHP 8.1.20

Operating System

No response

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 18, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 18, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 18, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 18, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 18, 2023
Initialize object.handlers to std_object_handlers on zend_objects_new. This
avoids a use-after-free for objects using custom handlers that are installed
after allocation, accessing the handlers on shutdown when they haven't been set
yet.

Fixes phpGH-11734
@iluuu1994
Copy link
Member

@tstarling Thanks for your analysis! I went for a slight variation of what you suggested in #11739, also setting object.handlers for objects that will later overwrite them. This avoids an additional check during freeing for each object.

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 18, 2023
Initialize object.handlers to std_object_handlers in zend_object_alloc. This
avoids a use-after-free for objects using custom handlers that are installed
after allocation, accessing the handlers on shutdown when they haven't been set
yet.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 18, 2023
Initialize object.handlers to std_object_handlers in zend_object_alloc. This
avoids a use-after-free for objects using custom handlers that are installed
after allocation, accessing the handlers on shutdown when they haven't been set
yet.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 18, 2023
Initialize object.handlers to std_object_handlers in zend_object_alloc. This
avoids a use-after-free for objects using custom handlers that are installed
after allocation, accessing the handlers on shutdown when they haven't been set
yet.

Fixes phpGH-11734
@Darshan1325
Copy link

/*

/* Add this include at the top of your PHP extension file */
#include "ext/spl/spl_heap.h"

/* Add this function to initialize the handlers /
static void initialize_heap_handlers() {
zend_object_handlers
heap_handlers = zend_get_std_object_handlers();
memcpy(&spl_heap_object_handlers, heap_handlers, sizeof(zend_object_handlers));

/* Now you can initialize any specific handlers for SplHeap here if needed */
/* For example, you can set custom implementations for SplHeap methods */
/* spl_heap_object_handlers.offset might be required to be adjusted */

}

/* Replace spl_heap_object_new_ex with this fixed version /
static zend_object
fixed_spl_heap_object_new_ex(zend_class_entry* ce, spl_heap_object** ptr)
{
spl_heap_object* intern;

intern = (spl_heap_object*)ecalloc(1, sizeof(spl_heap_object) + zend_object_properties_size(ce));
if (ptr) {
    *ptr = intern;
}

zend_object_std_init(&intern->std, ce);
object_properties_init(&intern->std, ce);

/* Fix: Call the initialize_heap_handlers function to ensure handlers are properly initialized */
initialize_heap_handlers();

intern->std.handlers = &spl_heap_object_handlers;
return &intern->std;

}

/* Replace the original spl_heap_object_new_ex implementation with our fixed version */
#define spl_heap_object_new_ex fixed_spl_heap_object_new_ex

/* The rest of your extension code goes here /
/
... */

/* Don't forget to implement the MINIT and MSHUTDOWN functions in your extension, if necessary. */

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jul 31, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 2, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 2, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 2, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 2, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 2, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 3, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 3, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 3, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 7, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 7, 2023
We should initialize object handlers before trying to allocate more memory,
because if we OOM we'll look for the free_obj handler which has not been
initialized. Furthermore, free_obj can run before the object is fully
initialized, requiring guards for potentially unset fields.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 8, 2023
We may OOM during object initialization. In this case, free_obj needs to guard
against NULL values. There may be more cases where this is an issue, these were
the ones I was able to discover via script.

Fixes phpGH-11734
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Aug 9, 2023
We may OOM during object initialization. In this case, free_obj needs to guard
against NULL values. There may be more cases where this is an issue, these were
the ones I was able to discover via script.

Fixes phpGH-11734
jorgsowa pushed a commit to jorgsowa/php-src that referenced this issue Aug 16, 2023
We may OOM during object initialization. In this case, free_obj needs to guard
against NULL values. There may be more cases where this is an issue, these were
the ones I was able to discover via script.

Fixes phpGH-11734
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants