From fe2dc2b4810167e08de5dae20f8d914f109c7a81 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Fri, 3 Feb 2023 09:17:33 -0500 Subject: [PATCH] Avoid crash for reset/end/next/prev() on ffi classes (#9711) (And any PECLs returning `zend_empty_array` in the handler->get_properties overrides) Closes GH-9697 This is similar to the fix used in d9651a941915eb5fb5ad557090b65256fd8509b6 for array_walk. This should make it safer for php-src (and PECLs, long-term) to return the empty immutable array in `handler->get_properties` to avoid wasting memory. See https://github.com/php/php-src/issues/9697#issuecomment-1273613175 The only possible internal iterator position for the empty array is at the end of the empty array (nInternalPointer=0). The `zend_hash*del*` helpers will always set nInternalPointer to 0 when an array becomes empty, regardless of previous insertions/deletions/updates to the array. --- NEWS | 2 ++ ext/ffi/tests/arrayPointer.phpt | 20 ++++++++++++++++++++ ext/standard/array.c | 16 ++++++++++++++++ 3 files changed, 38 insertions(+) create mode 100644 ext/ffi/tests/arrayPointer.phpt diff --git a/NEWS b/NEWS index ca6ee38b722..f16e815291a 100644 --- a/NEWS +++ b/NEWS @@ -27,6 +27,8 @@ PHP NEWS . Fixed bug GH-10292 (Made the default value of the first param of srand() and mt_srand() unknown). (kocsismate) . Fix incorrect check in cs_8559_5 in map_from_unicode(). (nielsdos) + . Fix bug GH-9697 for reset/end/next/prev() attempting to move pointer of + properties table for certain internal classes such as FFI classes 02 Feb 2023, PHP 8.1.15 diff --git a/ext/ffi/tests/arrayPointer.phpt b/ext/ffi/tests/arrayPointer.phpt new file mode 100644 index 00000000000..3a2b06563ad --- /dev/null +++ b/ext/ffi/tests/arrayPointer.phpt @@ -0,0 +1,20 @@ +--TEST-- +FFI: Test deprecated use of array helper functions on FFI classes doesn't crash +--EXTENSIONS-- +ffi +--INI-- +ffi.enable=1 +--FILE-- + +--EXPECTF-- +bool(false) +bool(false) +bool(false) +bool(false) diff --git a/ext/standard/array.c b/ext/standard/array.c index 3b807c739a8..9a814ea07da 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1073,6 +1073,10 @@ PHP_FUNCTION(end) ZEND_PARSE_PARAMETERS_END(); HashTable *array = get_ht_for_iap(array_zv, /* separate */ true); + if (zend_hash_num_elements(array) == 0) { + /* array->nInternalPointer is already 0 if the array is empty, even after removing elements */ + RETURN_FALSE; + } zend_hash_internal_pointer_end(array); if (USED_RET()) { @@ -1100,6 +1104,10 @@ PHP_FUNCTION(prev) ZEND_PARSE_PARAMETERS_END(); HashTable *array = get_ht_for_iap(array_zv, /* separate */ true); + if (zend_hash_num_elements(array) == 0) { + /* array->nInternalPointer is already 0 if the array is empty, even after removing elements */ + RETURN_FALSE; + } zend_hash_move_backwards(array); if (USED_RET()) { @@ -1127,6 +1135,10 @@ PHP_FUNCTION(next) ZEND_PARSE_PARAMETERS_END(); HashTable *array = get_ht_for_iap(array_zv, /* separate */ true); + if (zend_hash_num_elements(array) == 0) { + /* array->nInternalPointer is already 0 if the array is empty, even after removing elements */ + RETURN_FALSE; + } zend_hash_move_forward(array); if (USED_RET()) { @@ -1154,6 +1166,10 @@ PHP_FUNCTION(reset) ZEND_PARSE_PARAMETERS_END(); HashTable *array = get_ht_for_iap(array_zv, /* separate */ true); + if (zend_hash_num_elements(array) == 0) { + /* array->nInternalPointer is already 0 if the array is empty, even after removing elements */ + RETURN_FALSE; + } zend_hash_internal_pointer_reset(array); if (USED_RET()) {