diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-11-13 12:52:29 +0100 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2018-11-13 14:16:27 +0000 |
commit | c2aeb1c6d21cd952be6b2c1ed765e4c5f0b2d340 (patch) | |
tree | 439e5450914528b7268f101ca0311b2d03e6014f | |
parent | 071af956fdf4958ad8a5148c219878295864578d (diff) | |
download | qtwebengine-chromium-c2aeb1c6d21cd952be6b2c1ed765e4c5f0b2d340.tar.gz |
[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>
-rw-r--r-- | chromium/v8/src/js/array.js | 1 | ||||
-rw-r--r-- | chromium/v8/src/runtime/runtime-array.cc | 12 |
2 files changed, 12 insertions, 1 deletions
diff --git a/chromium/v8/src/js/array.js b/chromium/v8/src/js/array.js index 35db963d183..f5617919a86 100644 --- a/chromium/v8/src/js/array.js +++ b/chromium/v8/src/js/array.js @@ -793,6 +793,7 @@ function InnerArraySort(array, length, comparefn) { // array and move the undefineds after that. Holes are removed. // This happens for Array as well as non-Array objects. var num_non_undefined = %PrepareElementsForSort(array, length); + assert(num_non_undefined <= length); QuickSort(array, 0, num_non_undefined); diff --git a/chromium/v8/src/runtime/runtime-array.cc b/chromium/v8/src/runtime/runtime-array.cc index ae23c99910b..9f2442812fb 100644 --- a/chromium/v8/src/runtime/runtime-array.cc +++ b/chromium/v8/src/runtime/runtime-array.cc @@ -151,7 +151,15 @@ Object* RemoveArrayHolesGeneric(Isolate* isolate, Handle<JSReceiver> receiver, MAYBE_RETURN(delete_result, ReadOnlyRoots(isolate).exception()); } - 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 @@ -168,6 +176,7 @@ Object* RemoveArrayHoles(Isolate* isolate, Handle<JSReceiver> receiver, Handle<JSObject> object = Handle<JSObject>::cast(receiver); if (object->HasStringWrapperElements()) { int len = String::cast(Handle<JSValue>::cast(object)->value())->length(); + DCHECK_LE(len, limit); return Smi::FromInt(len); } @@ -290,6 +299,7 @@ Object* RemoveArrayHoles(Isolate* isolate, Handle<JSReceiver> receiver, } } + DCHECK_LE(result, limit); return *isolate->factory()->NewNumberFromUint(result); } |