Fix GH-15833: Segmentation fault (access null pointer) in ext/spl/spl_array.c #17235
Conversation
…spl_array.c We're accessing the object properties table directly in spl, but we're not accounting for lazy objects. Upon accessing we should trigger the initialization as spl is doing direct manipulations on the object property table and expects a real object.
| /* Since we're directly playing with the properties table, we shall initialize the lazy object directly. | ||
| * If we don't, it's possible to continue working with the wrong object in case we're using a proxy. */ | ||
| if (UNEXPECTED(zend_lazy_object_must_init(obj))) { | ||
| obj = zend_lazy_object_init(obj); |
There was a problem hiding this comment.
zend_lazy_object_init() may return NULL when initialization fails (in which case EG(exception) is set, too).
Unfortunately I don't see other solution than to return NULL from spl_array_get_hash_table_ptr and to handle that in every caller. (An alternative would be to return a ptr-ptr to an empty array, and to rely on the exception to discard the result of the operation, but then we need to manage that array somehow.)
There was a problem hiding this comment.
Ouch, okay, I'll do the NULL thing then :/
There was a problem hiding this comment.
Actually I believe we can just store the empty array in intern->array. I pushed a commit that does that, so the total patch stays simple. The only downside is that any further action won't do anything on the object, which may be fine as the object failed initialization anyway. What do you think?
There was a problem hiding this comment.
The only downside is that any further action won't do anything on the object
I would prefer if further actions on the ArrayObject was carried normally, causing a new attempt to initialize the object and probably a new exception. Otherwise the silent failure could be surprising.
Storing the empty array in a new field in spl_array_object would be fine I think.
There was a problem hiding this comment.
Done. I also added some checks to avoid unexpected modification to the sentinel
We're accessing the object properties table directly in spl, but we're not accounting for lazy objects. Upon accessing we should trigger the initialization as spl is doing direct manipulations on the object property table and expects a real object.