From de38764e889ae6624fa09ef09efea55c6670e325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Csaba=20Osztrogon=C3=A1c?= Date: Fri, 25 Sep 2020 15:06:29 +0200 Subject: [PATCH] Fix heap buffer overflow in Array.prototype.copyWithin (#4211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 2nd and 3rd argument evaluation of Array.prototype.copyWithin can change the length of the array as a side-effect. But ES11 spec says that the algorithm should use the original length. In this case it could happen that the underlying buffer should be extended. Fixes #4204 JerryScript-DCO-1.0-Signed-off-by: Csaba Osztrogonác csaba.osztrogonac@h-lab.eu --- .../ecma-builtin-array-prototype.c | 10 +++++----- .../jerry/es.next/array-prototype-copywithin.js | 17 ++++++++++++++--- .../jerry/es.next/regression-test-issue-4146.js | 9 +++++---- tests/test262-esnext-excludelist.xml | 2 -- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/jerry-core/ecma/builtin-objects/ecma-builtin-array-prototype.c b/jerry-core/ecma/builtin-objects/ecma-builtin-array-prototype.c index 82532a667..7d28e190d 100644 --- a/jerry-core/ecma/builtin-objects/ecma-builtin-array-prototype.c +++ b/jerry-core/ecma/builtin-objects/ecma-builtin-array-prototype.c @@ -2491,14 +2491,13 @@ ecma_builtin_array_prototype_object_copy_within (const ecma_value_t args[], /**< } } - if (target >= len || start >= end || end == 0) + ecma_length_t count = JERRY_MIN (end - start, len - target); + if (end <= start || len <= target) /* count <= 0 check, but variables are unsigned */ { ecma_ref_object (obj_p); return ecma_make_object_value (obj_p); } - ecma_length_t count = JERRY_MIN (end - start, len - target); - bool forward = true; if (start < target && target < start + count) @@ -2511,12 +2510,13 @@ ecma_builtin_array_prototype_object_copy_within (const ecma_value_t args[], /**< if (ecma_op_object_is_fast_array (obj_p)) { ecma_extended_object_t *ext_obj_p = (ecma_extended_object_t *) obj_p; + const uint32_t actual_length = ext_obj_p->u.array.length; - if (ext_obj_p->u.array.u.hole_count < ECMA_FAST_ARRAY_HOLE_ONE) + if (ext_obj_p->u.array.u.hole_count < ECMA_FAST_ARRAY_HOLE_ONE + && ((forward && (target + count - 1 < actual_length)) || (!forward && (target < actual_length)))) { if (obj_p->u1.property_list_cp != JMEM_CP_NULL) { - count = JERRY_MIN (ext_obj_p->u.array.length, count); ecma_value_t *buffer_p = ECMA_GET_NON_NULL_POINTER (ecma_value_t, obj_p->u1.property_list_cp); for (; count > 0; count--) diff --git a/tests/jerry/es.next/array-prototype-copywithin.js b/tests/jerry/es.next/array-prototype-copywithin.js index ddf1ed4db..2d5dab162 100644 --- a/tests/jerry/es.next/array-prototype-copywithin.js +++ b/tests/jerry/es.next/array-prototype-copywithin.js @@ -95,7 +95,6 @@ var value = array.copyWithin(0, { array.length = 0; } }) - array_check(value, []); // Extend the buffer @@ -105,7 +104,6 @@ var value = array.copyWithin(1, { array.length = 6; } }) - array_check(value, [1, 1, 2, undefined, undefined, undefined]); // Reduce the buffer @@ -115,5 +113,18 @@ var value = array.copyWithin(4, 2, { array.length = 3; } }) - array_check(value, [1, 2, 3]); + +// Reduce the buffer and extend the buffer +var array = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; +var value = array.copyWithin(7, { + valueOf: function() { + array.length = 5; + } +}) +array_check(value, [1, 2, 3, 4, 5, , , 1, 2, 3]); + +// Copy with overlapping (backward copy) +var array = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; +var value = array.copyWithin(0, 2, 8) +array_check(value, [3, 4, 5, 6, 7, 8, 7, 8, 9, 10]); diff --git a/tests/jerry/es.next/regression-test-issue-4146.js b/tests/jerry/es.next/regression-test-issue-4146.js index f136f8b28..bc5548cbb 100644 --- a/tests/jerry/es.next/regression-test-issue-4146.js +++ b/tests/jerry/es.next/regression-test-issue-4146.js @@ -32,7 +32,7 @@ Array.prototype.equals = function (array) { function longDenseArray(){ var a = [0]; - for(var i = 0; i < 200; i++){ + for(var i = 0; i < 60; i++){ a[i] = i; } return a; @@ -43,7 +43,8 @@ function shorten(){ return 1; } +var array = [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,,,,,,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19]; var currArray = longDenseArray(); -assert (currArray.copyWithin (200, {valueOf: shorten}).length == 20) -var array = [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19]; -assert (currArray.copyWithin(200, {valueOf: shorten}).equals (array)) +currArray.copyWithin(25, {valueOf: shorten}) +assert (currArray.length == 44) +assert (currArray.equals (array)) diff --git a/tests/test262-esnext-excludelist.xml b/tests/test262-esnext-excludelist.xml index 894b3bb3b..086105064 100644 --- a/tests/test262-esnext-excludelist.xml +++ b/tests/test262-esnext-excludelist.xml @@ -142,8 +142,6 @@ - -