Fix GH-20042: SEGV in array.c when error handler clobbers IAP object

[iliaal]

Summary

• get_ht_for_iap() emits a deprecation for object IAP args, triggering the user error handler. If the handler clobbers the by-ref variable, the code reads a dead zval as an object pointer and segfaults.
• After the deprecation, re-check that the zval is still IS_OBJECT. Return NULL if clobbered. All 6 callers (end/prev/next/reset/current/key) handle NULL gracefully.
• Regression test covers all 6 functions.

Fixes #20042

[iluuu1994]

Hi @iliaal. As #20042 (comment) mentions, we generally don't try to fix set_error_handler() clobber issues anymore. In my opinion, this should be fixed in a general way (i.e. something like GH-12805), or not at all.

[iliaal]

Hi @iliaal. As #20042 (comment) mentions, we generally don't try to fix set_error_handler() clobber issues anymore. In my opinion, this should be fixed in a general way (i.e. something like GH-12805), or not at all.

Any particular reason (for future reference)? That being said, the change is fairly benign and covers all the by-ref array functions.

[iluuu1994]

Mainly because there are endless ways in which set_error_handler() can cause similar issues, and hunting them down costs resources and hurts code readability to solve an artificial issue. The vast majority of these bugs are found by fuzzers, rather than users.

[iliaal]

Fair point, the only reason I tried to tackle it is cause of Segv.
I briefly look at the #12805 and complexity wise seems quite a bit more elaborate.

I'd say small point fixes (just a if (UNEXPECTED(Z_TYPE_P(zv) != IS_OBJECT)) { check in this case) that add basic guard might be ok, that being said I do appreciate where you are coming from, so I'll leave it to your discretion :-)

[iluuu1994]

I'd say small point fixes (just a if (UNEXPECTED(Z_TYPE_P(zv) != IS_OBJECT)) { check in this case) that add basic guard might be ok, that being said I do appreciate where you are coming from, so I'll leave it to your discretion :-)

IMO it's not worth the hassle, but I don't have any special privileges and won't object if somebody else would like to merge it. If more time is invested, it's probably better to try to finish GH-12805 so that this class of issues is solved completely. There are other similar classes (e.g. __toString(), __destruct, etc.), but the same applies there.