[builtins] Fix OOB read/write using Array.from
Always use the runtime to set the length on an array if it doesn't match
the expected length after populating it using Array.from.
Bug: chromium:821137
Change-Id: I5a730db58de61ba789040e6dfc815d6067fbae64
Reviewed-on: https://chromium-review.googlesource.com/962222
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51919}diff --git a/src/builtins/builtins-array-gen.cc b/src/builtins/builtins-array-gen.cc
index dcf3be4..3a74342 100644
--- a/src/builtins/builtins-array-gen.cc
+++ b/src/builtins/builtins-array-gen.cc
@@ -1945,10 +1945,13 @@
void GenerateSetLength(TNode<Context> context, TNode<Object> array,
TNode<Number> length) {
Label fast(this), runtime(this), done(this);
+ // TODO(delphick): We should be able to skip the fast set altogether, if the
+ // length already equals the expected length, which it always is now on the
+ // fast path.
// Only set the length in this stub if
// 1) the array has fast elements,
// 2) the length is writable,
- // 3) the new length is greater than or equal to the old length.
+ // 3) the new length is equal to the old length.
// 1) Check that the array has fast elements.
// TODO(delphick): Consider changing this since it does an an unnecessary
@@ -1970,10 +1973,10 @@
// BranchIfFastJSArray above.
EnsureArrayLengthWritable(LoadMap(fast_array), &runtime);
- // 3) If the created array already has a length greater than required,
+ // 3) If the created array's length does not match the required length,
// then use the runtime to set the property as that will insert holes
- // into the excess elements and/or shrink the backing store.
- GotoIf(SmiLessThan(length_smi, old_length), &runtime);
+ // into excess elements or shrink the backing store as appropriate.
+ GotoIf(SmiNotEqual(length_smi, old_length), &runtime);
StoreObjectFieldNoWriteBarrier(fast_array, JSArray::kLengthOffset,
length_smi);
diff --git a/test/mjsunit/regress/regress-821137.js b/test/mjsunit/regress/regress-821137.js
new file mode 100644
index 0000000..639b3b9
--- /dev/null
+++ b/test/mjsunit/regress/regress-821137.js
@@ -0,0 +1,27 @@
+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Tests that creating an iterator that shrinks the array populated by
+// Array.from does not lead to out of bounds writes.
+let oobArray = [];
+let maxSize = 1028 * 8;
+Array.from.call(function() { return oobArray }, {[Symbol.iterator] : _ => (
+ {
+ counter : 0,
+ next() {
+ let result = this.counter++;
+ if (this.counter > maxSize) {
+ oobArray.length = 0;
+ return {done: true};
+ } else {
+ return {value: result, done: false};
+ }
+ }
+ }
+) });
+assertEquals(oobArray.length, maxSize);
+
+// iterator reset the length to 0 just before returning done, so this will crash
+// if the backing store was not resized correctly.
+oobArray[oobArray.length - 1] = 0x41414141;
