Skip to content
Permalink
Browse files

Race Condition in arrayProtoFuncReverse() causes wrong results or crash

https://bugs.webkit.org/show_bug.cgi?id=183901

Reviewed by Keith Miller.

JSTests:

New test.

* stress/array-reverse-doesnt-clobber.js: Added.
(testArrayReverse):
(createArrayOfArrays):
(createArrayStorage):

Source/JavaScriptCore:

Added write barriers to ensure the reversed contents are properly marked.

* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncReverse):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@229850 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information...
msaboff@apple.com
msaboff@apple.com committed Mar 22, 2018
1 parent c7012f3 commit 4277697ef9384adea6f4c63ed1215a05990e85b4
@@ -1,3 +1,17 @@
2018-03-22 Michael Saboff <msaboff@apple.com>

Race Condition in arrayProtoFuncReverse() causes wrong results or crash
https://bugs.webkit.org/show_bug.cgi?id=183901

Reviewed by Keith Miller.

New test.

* stress/array-reverse-doesnt-clobber.js: Added.
(testArrayReverse):
(createArrayOfArrays):
(createArrayStorage):

2018-03-21 Filip Pizlo <fpizlo@apple.com>

ScopedArguments should do poisoning and index masking
@@ -0,0 +1,61 @@
// This tests that array.Prototype.reverse() doesn't inadvertently clobber indexed properties.
// This test shouldn't throw or crash.

const outerArrayLength = 10000;
const innerArrayLength = 128;

function testArrayReverse(createArray)
{
const limit = 5;
let save = [0, 0];

for (let at = 0; at < limit; at++) {
let arr = createArray();

let v = [];
for (let i = 0; i < 273; i++) {
for (let j = 0; j < 8; j++)
arr.reverse();

v.push(new String("X").repeat(123008));
}

for (let i = 0; i < arr.length; i++) {
if (arr[i].length != innerArrayLength)
throw "arr[" + i + "].length has changed from " + innerArrayLength + " to " + arr[i].length;
}

let f = [];
for (let i = 0; i < 1000; i++)
f.push(new Array(16).fill(0x42424242));

save.push(arr);
save.push(v);
save.push(f);
}
}

function createArrayOfArrays()
{
let result = new Array(outerArrayLength);

for (let i = 0; i < result.length; i++)
result[i] = new Array(innerArrayLength).fill(0x41414141);

return result;
}

var alt = 0;

function createArrayStorage()
{
let result = createArrayOfArrays();

if (!(typeof ensureArrayStorage === undefined) && alt++ % 0)
ensureArrayStorage(result);

return result;
}

testArrayReverse(createArrayOfArrays);
testArrayReverse(createArrayStorage);
@@ -1,3 +1,15 @@
2018-03-22 Michael Saboff <msaboff@apple.com>

Race Condition in arrayProtoFuncReverse() causes wrong results or crash
https://bugs.webkit.org/show_bug.cgi?id=183901

Reviewed by Keith Miller.

Added write barriers to ensure the reversed contents are properly marked.

* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncReverse):

2018-03-21 Filip Pizlo <fpizlo@apple.com>

ScopedArguments should do poisoning and index masking
@@ -839,6 +839,8 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncReverse(ExecState* exec)
if (containsHole(data, length) && holesMustForwardToPrototype(vm, thisObject))
break;
std::reverse(data, data + length);
if (!hasInt32(thisObject->indexingType()))
vm.heap.writeBarrier(thisObject);
return JSValue::encode(thisObject);
}
case ALL_DOUBLE_INDEXING_TYPES: {
@@ -859,6 +861,7 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncReverse(ExecState* exec)
break;
auto data = storage.vector().data();
std::reverse(data, data + length);
vm.heap.writeBarrier(thisObject);
return JSValue::encode(thisObject);
}
}

0 comments on commit 4277697

Please sign in to comment.
You can’t perform that action at this time.