From dfb173bafb2d7a22e1cf0c444e749c1dab54832b Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 16 Jun 2022 18:25:48 +0200 Subject: [PATCH 1/2] Avoid wrap-around in array_fill() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `start_key + num` is supposed to fit into a `zend_long`, so we should not allow to pass values which won't. We also fix the `UNEXPECTED(EXPECTED(…))`, which does not make sense, and replace the magic number with the respective macro. --- ext/standard/array.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index a3ce0d6c80cf7..80dcf02f6fc82 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2598,10 +2598,10 @@ PHP_FUNCTION(array_fill) ZEND_PARSE_PARAMETERS_END(); if (EXPECTED(num > 0)) { - if (sizeof(num) > 4 && UNEXPECTED(EXPECTED(num > 0x7fffffff))) { + if (sizeof(num) > 4 && UNEXPECTED(num >INT_MAX)) { zend_argument_value_error(2, "is too large"); RETURN_THROWS(); - } else if (UNEXPECTED(start_key > ZEND_LONG_MAX - num + 1)) { + } else if (UNEXPECTED(start_key > ZEND_LONG_MAX - num)) { zend_throw_error(NULL, "Cannot add element to the array as the next element is already occupied"); RETURN_THROWS(); } else if (EXPECTED(start_key >= 0) && EXPECTED(start_key < num)) { From 1298d66343195797c852fe8b814e965a67a95ab9 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 20 Jun 2022 12:50:51 +0200 Subject: [PATCH 2/2] Do not avoid wrap-around of .nNextFreeElement This is by design, and is supposed to have the same behavior like doing the operations manually. We add a test to verify this behavior. --- ext/standard/array.c | 2 +- .../tests/array/array_fill_variation6.phpt | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 ext/standard/tests/array/array_fill_variation6.phpt diff --git a/ext/standard/array.c b/ext/standard/array.c index 80dcf02f6fc82..2514beee62bde 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -2601,7 +2601,7 @@ PHP_FUNCTION(array_fill) if (sizeof(num) > 4 && UNEXPECTED(num >INT_MAX)) { zend_argument_value_error(2, "is too large"); RETURN_THROWS(); - } else if (UNEXPECTED(start_key > ZEND_LONG_MAX - num)) { + } else if (UNEXPECTED(start_key > ZEND_LONG_MAX - num + 1)) { zend_throw_error(NULL, "Cannot add element to the array as the next element is already occupied"); RETURN_THROWS(); } else if (EXPECTED(start_key >= 0) && EXPECTED(start_key < num)) { diff --git a/ext/standard/tests/array/array_fill_variation6.phpt b/ext/standard/tests/array/array_fill_variation6.phpt new file mode 100644 index 0000000000000..60dcec7e231ea --- /dev/null +++ b/ext/standard/tests/array/array_fill_variation6.phpt @@ -0,0 +1,19 @@ +--TEST-- +array_fill(): last element +--FILE-- +getMessage(), PHP_EOL; +} +?> +--EXPECT-- +int(1) +bool(true) +Cannot add element to the array as the next element is already occupied