summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAllan Sandfeld Jensen <allan.jensen@qt.io>2018-11-13 12:52:29 +0100
committerAllan Sandfeld Jensen <allan.jensen@qt.io>2018-11-14 12:07:47 +0000
commit37195be7fc4d1d4b773f1cb466e2520dd8d6d030 (patch)
treea3e8b614b2e2813e034ae00f8fefe46cf11defbb
parent9f4aba186947a505293d2414e40a5080a2c52861 (diff)
[Backport] Fix for CVE-2018-17478
Merged: [array] Ensure PrepareElementsForSort returns a legal value PrepareElementsForSort must return a number less than or equal the array length. No-Try: true No-Presubmit: true No-Treechecks: true Bug: chromium:897512, v8:7382 Change-Id: If5f9c4d052e623ab9f3300b8534603abbee859fa Reviewed-on: https://chromium-review.googlesource.com/c/1297958 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Camillo Bruni <cbruni@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#56982} Reviewed-on: https://chromium-review.googlesource.com/c/1304354 Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/branch-heads/7.0@{#67} Cr-Branched-From: 6e2adae6f7f8e891cfd01f3280482b20590427a6-refs/heads/7.0.276@{#1} Cr-Branched-From: bc08a8624cbbea7a2d30071472bc73ad9544eadf-refs/heads/master@{#55424} Reviewed-by: Michael BrĂ¼ning <michael.bruning@qt.io> Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
-rw-r--r--chromium/v8/src/runtime/runtime-array.cc12
1 files changed, 11 insertions, 1 deletions
diff --git a/chromium/v8/src/runtime/runtime-array.cc b/chromium/v8/src/runtime/runtime-array.cc
index f07c842baec..5fc804d3a74 100644
--- a/chromium/v8/src/runtime/runtime-array.cc
+++ b/chromium/v8/src/runtime/runtime-array.cc
@@ -109,7 +109,15 @@ Object* PrepareSlowElementsForSort(Handle<JSObject> object, uint32_t limit) {
new_dict->UpdateMaxNumberKey(max_key, object);
JSObject::ValidateElements(*object);
- return *isolate->factory()->NewNumberFromUint(result);
+ // TODO(jgruber, szuend, chromium:897512): This is a workaround to prevent
+ // returning a number greater than array.length to Array.p.sort, which could
+ // trigger OOB accesses. There is still a correctness bug here though in
+ // how we shift around undefineds and delete elements in the two blocks above.
+ // This needs to be fixed soon.
+ const uint32_t number_of_non_undefined_elements = std::min(limit, result);
+
+ return *isolate->factory()->NewNumberFromUint(
+ number_of_non_undefined_elements);
}
// Collects all defined (non-hole) and non-undefined (array) elements at the
@@ -123,6 +131,7 @@ Object* PrepareElementsForSort(Handle<JSObject> object, uint32_t limit) {
}
if (object->HasStringWrapperElements()) {
int len = String::cast(Handle<JSValue>::cast(object)->value())->length();
+ DCHECK_LE(len, limit);
return Smi::FromInt(len);
}
@@ -241,6 +250,7 @@ Object* PrepareElementsForSort(Handle<JSObject> object, uint32_t limit) {
}
}
+ DCHECK_LE(result, limit);
return *isolate->factory()->NewNumberFromUint(result);
}