diff --git a/NEWS b/NEWS index 2d0abd7eb42f..97bca98512d4 100644 --- a/NEWS +++ b/NEWS @@ -85,6 +85,8 @@ PHP NEWS (Weilin Du) . Fixed integer overflow in getimagesize() and getimagesizefromstring() when parsing an IFF chunk with a size of INT_MAX. (David Carlier, Weilin Du) + . Fixed bug GH-22678 (Use-after-free in array_multisort() when the comparator + mutates the array being sorted). (iliaal) - Zip: . Fixed bug GH-22649 (ZipArchive::setCommentName() and setCommentIndex() diff --git a/ext/standard/array.c b/ext/standard/array.c index 13731592d836..0f887406ce94 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -6046,6 +6046,7 @@ PHP_FUNCTION(array_multisort) { zval* args; zval** arrays; + HashTable** hashes; Bucket** indirect; uint32_t idx; HashTable* hash; @@ -6167,11 +6168,17 @@ PHP_FUNCTION(array_multisort) for (i = 0; i < array_size; i++) { indirect[i] = indirects + (i * (num_arrays + 1)); } + hashes = (HashTable **)safe_emalloc(num_arrays, sizeof(HashTable *), 0); + for (i = 0; i < num_arrays; i++) { + hashes[i] = Z_ARRVAL_P(arrays[i]); + GC_ADDREF(hashes[i]); + HT_ALLOW_COW_VIOLATION(hashes[i]); + } for (i = 0; i < num_arrays; i++) { k = 0; - if (HT_IS_PACKED(Z_ARRVAL_P(arrays[i]))) { - zval *zv = Z_ARRVAL_P(arrays[i])->arPacked; - for (idx = 0; idx < Z_ARRVAL_P(arrays[i])->nNumUsed; idx++, zv++) { + if (HT_IS_PACKED(hashes[i])) { + zval *zv = hashes[i]->arPacked; + for (idx = 0; idx < hashes[i]->nNumUsed; idx++, zv++) { if (Z_TYPE_P(zv) == IS_UNDEF) continue; ZVAL_COPY_VALUE(&indirect[k][i].val, zv); indirect[k][i].h = idx; @@ -6179,8 +6186,8 @@ PHP_FUNCTION(array_multisort) k++; } } else { - Bucket *p = Z_ARRVAL_P(arrays[i])->arData; - for (idx = 0; idx < Z_ARRVAL_P(arrays[i])->nNumUsed; idx++, p++) { + Bucket *p = hashes[i]->arData; + for (idx = 0; idx < hashes[i]->nNumUsed; idx++, p++) { if (Z_TYPE(p->val) == IS_UNDEF) continue; indirect[k][i] = *p; k++; @@ -6200,7 +6207,7 @@ PHP_FUNCTION(array_multisort) /* Restructure the arrays based on sorted indirect - this is mostly taken from zend_hash_sort() function. */ for (i = 0; i < num_arrays; i++) { - hash = Z_ARRVAL_P(arrays[i]); + hash = hashes[i]; hash->nNumUsed = array_size; hash->nNextFreeElement = array_size; hash->nInternalPointer = 0; @@ -6229,6 +6236,14 @@ PHP_FUNCTION(array_multisort) RETVAL_TRUE; clean_up: + for (i = 0; i < num_arrays; i++) { + if (UNEXPECTED(GC_DELREF(hashes[i]) == 0)) { + zend_array_destroy(hashes[i]); + } else { + gc_check_possible_root((zend_refcounted *)hashes[i]); + } + } + efree(hashes); efree(indirects); efree(indirect); efree(func); diff --git a/ext/standard/tests/array/gh22678.phpt b/ext/standard/tests/array/gh22678.phpt new file mode 100644 index 000000000000..c1b71b592520 --- /dev/null +++ b/ext/standard/tests/array/gh22678.phpt @@ -0,0 +1,48 @@ +--TEST-- +GH-22678 (Use-after-free in array_multisort() when the comparator mutates the array) +--FILE-- + 'c', 3 => 'a', 1 => 'b']; +array_multisort($b, SORT_STRING); +echo "repacked: ", implode(',', $b), "\n"; + +class Boom { + public function __toString(): string { + throw new Exception("boom"); + } +} + +$c = [new Boom(), new Boom()]; +try { + array_multisort($c, SORT_STRING); +} catch (Exception $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; +} +?> +--EXPECT-- +freed mid-sort: bool(true) +repacked: a,b,c +Exception: boom