From ebf2abf58b26bc4854d0af5be62ec7092059d5b2 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 2 May 2025 07:11:57 +0100 Subject: [PATCH 1/4] Fix GH-18480: array_splice overflow on array length with offset. --- ext/standard/array.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index b89deb241f046..017f6a5139335 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3252,7 +3252,7 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H /* If hash for removed entries exists, go until offset+length and copy the entries to it */ if (removed != NULL) { - for ( ; pos < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { + for ( ; length <= ZEND_LONG_MAX - offset && pos < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { if (Z_TYPE_P(entry) == IS_UNDEF) continue; pos++; Z_TRY_ADDREF_P(entry); @@ -3260,9 +3260,9 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H zend_hash_packed_del_val(in_hash, entry); } } else { /* otherwise just skip those entries */ - int pos2 = pos; + zend_long pos2 = pos; - for ( ; pos2 < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { + for ( ; length <= ZEND_LONG_MAX - offset && pos2 < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { if (Z_TYPE_P(entry) == IS_UNDEF) continue; pos2++; zend_hash_packed_del_val(in_hash, entry); From d3b74ba3bb3bce9e658bbc15c7fde013b48de84b Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 2 May 2025 11:15:43 +0100 Subject: [PATCH 2/4] let s try another approach and let's finish earlier just enough to create the return value. --- ext/standard/array.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 017f6a5139335..a92ef2717550c 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3234,6 +3234,10 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H /* Create and initialize output hash */ zend_hash_init(&out_hash, (length > 0 ? num_in - length : 0) + (replace ? zend_hash_num_elements(replace) : 0), NULL, ZVAL_PTR_DTOR, 0); + if (length > ZEND_LONG_MAX - offset) { + goto end; + } + if (HT_IS_PACKED(in_hash)) { /* Start at the beginning of the input hash and copy entries to output hash until offset is reached */ entry = in_hash->arPacked; @@ -3252,7 +3256,7 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H /* If hash for removed entries exists, go until offset+length and copy the entries to it */ if (removed != NULL) { - for ( ; length <= ZEND_LONG_MAX - offset && pos < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { + for ( ; pos < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { if (Z_TYPE_P(entry) == IS_UNDEF) continue; pos++; Z_TRY_ADDREF_P(entry); @@ -3262,7 +3266,7 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H } else { /* otherwise just skip those entries */ zend_long pos2 = pos; - for ( ; length <= ZEND_LONG_MAX - offset && pos2 < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { + for ( ; pos2 < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { if (Z_TYPE_P(entry) == IS_UNDEF) continue; pos2++; zend_hash_packed_del_val(in_hash, entry); @@ -3368,6 +3372,7 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H } } +end: /* replace HashTable data */ HT_SET_ITERATORS_COUNT(&out_hash, HT_ITERATORS_COUNT(in_hash)); HT_SET_ITERATORS_COUNT(in_hash, 0); From 17677def254bfbf3a1f214fa680ca40998771e4a Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 2 May 2025 18:27:19 +0100 Subject: [PATCH 3/4] add tests --- ext/standard/array.c | 2 +- ext/standard/tests/array/gh18480.phpt | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 ext/standard/tests/array/gh18480.phpt diff --git a/ext/standard/array.c b/ext/standard/array.c index a92ef2717550c..48dbda4f5468f 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3234,7 +3234,7 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H /* Create and initialize output hash */ zend_hash_init(&out_hash, (length > 0 ? num_in - length : 0) + (replace ? zend_hash_num_elements(replace) : 0), NULL, ZVAL_PTR_DTOR, 0); - if (length > ZEND_LONG_MAX - offset) { + if (length > ZEND_LONG_MAX - offset || length < ZEND_LONG_MIN + offset) { goto end; } diff --git a/ext/standard/tests/array/gh18480.phpt b/ext/standard/tests/array/gh18480.phpt new file mode 100644 index 0000000000000..4665cfea6a755 --- /dev/null +++ b/ext/standard/tests/array/gh18480.phpt @@ -0,0 +1,15 @@ +--TEST-- +GH-18480 (array_splice overflow with large offset / length values) +--FILE-- + Date: Sun, 4 May 2025 12:23:08 +0100 Subject: [PATCH 4/4] changes from feedback --- ext/standard/array.c | 15 ++++------- ext/standard/tests/array/gh18480.phpt | 39 ++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 48dbda4f5468f..4d3c03bce5262 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3234,10 +3234,6 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H /* Create and initialize output hash */ zend_hash_init(&out_hash, (length > 0 ? num_in - length : 0) + (replace ? zend_hash_num_elements(replace) : 0), NULL, ZVAL_PTR_DTOR, 0); - if (length > ZEND_LONG_MAX - offset || length < ZEND_LONG_MIN + offset) { - goto end; - } - if (HT_IS_PACKED(in_hash)) { /* Start at the beginning of the input hash and copy entries to output hash until offset is reached */ entry = in_hash->arPacked; @@ -3256,7 +3252,7 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H /* If hash for removed entries exists, go until offset+length and copy the entries to it */ if (removed != NULL) { - for ( ; pos < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { + for ( ; pos - offset < length && idx < in_hash->nNumUsed; idx++, entry++) { if (Z_TYPE_P(entry) == IS_UNDEF) continue; pos++; Z_TRY_ADDREF_P(entry); @@ -3266,7 +3262,7 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H } else { /* otherwise just skip those entries */ zend_long pos2 = pos; - for ( ; pos2 < offset + length && idx < in_hash->nNumUsed; idx++, entry++) { + for ( ; pos2 - offset < length && idx < in_hash->nNumUsed; idx++, entry++) { if (Z_TYPE_P(entry) == IS_UNDEF) continue; pos2++; zend_hash_packed_del_val(in_hash, entry); @@ -3321,7 +3317,7 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H /* If hash for removed entries exists, go until offset+length and copy the entries to it */ if (removed != NULL) { - for ( ; pos < offset + length && idx < in_hash->nNumUsed; idx++, p++) { + for ( ; pos - offset < length && idx < in_hash->nNumUsed; idx++, p++) { if (Z_TYPE(p->val) == IS_UNDEF) continue; pos++; entry = &p->val; @@ -3334,9 +3330,9 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H zend_hash_del_bucket(in_hash, p); } } else { /* otherwise just skip those entries */ - int pos2 = pos; + zend_long pos2 = pos; - for ( ; pos2 < offset + length && idx < in_hash->nNumUsed; idx++, p++) { + for ( ; pos2 - offset < length && idx < in_hash->nNumUsed; idx++, p++) { if (Z_TYPE(p->val) == IS_UNDEF) continue; pos2++; zend_hash_del_bucket(in_hash, p); @@ -3372,7 +3368,6 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H } } -end: /* replace HashTable data */ HT_SET_ITERATORS_COUNT(&out_hash, HT_ITERATORS_COUNT(in_hash)); HT_SET_ITERATORS_COUNT(in_hash, 0); diff --git a/ext/standard/tests/array/gh18480.phpt b/ext/standard/tests/array/gh18480.phpt index 4665cfea6a755..b9466029d4ee9 100644 --- a/ext/standard/tests/array/gh18480.phpt +++ b/ext/standard/tests/array/gh18480.phpt @@ -2,14 +2,39 @@ GH-18480 (array_splice overflow with large offset / length values) --FILE-- PHP_INT_MAX]; + $offset = PHP_INT_MAX; + var_dump(array_splice($a,$offset, $length)); + $a = ["a" => PHP_INT_MAX]; + $offset = PHP_INT_MIN; + var_dump(array_splice($a,$offset, $length)); +} +--EXPECTF-- array(0) { } array(0) { } - +array(0) { +} +array(0) { +} +array(0) { +} +array(1) { + [0]=> + int(%d) +} +array(0) { +} +array(1) { + ["a"]=> + int(%d) +}