Skip to content
Permalink
Browse files

Assert in JavascriptArray::EntryReverse when length > Uint32Max

Turns out it's pretty easy to make the length property of a TypedArray greater than Uint32Max. Passing a TypedArray object like that to Array#reverse hits an assert verifying the length is <= Uint32Max.

This is a valid situation to get into, though, so instead of asserting we should fall into the object (non-Array, non-TypedArray) case and use the slow path which supports larger lengths.

Fixes #6179
  • Loading branch information...
boingoing committed Jun 27, 2019
1 parent b83edb0 commit d404837eac03ce36ad508a3efbfc0eb6c388a72d
Showing with 69 additions and 4 deletions.
  1. +2 −3 lib/Runtime/Library/JavascriptArray.cpp
  2. +60 −0 test/Bugs/bug_6179.js
  3. +7 −1 test/Bugs/rlexe.xml
@@ -5533,7 +5533,6 @@ using namespace Js;
{
JS_REENTRANT_UNLOCK(jsReentLock, return JavascriptArray::ReverseHelper(pArr, nullptr, obj, length.GetSmallIndex(), scriptContext));
}
Assert(pArr == nullptr || length.IsUint32Max()); // if pArr is not null lets make sure length is safe to cast, which will only happen if length is a uint32max

JS_REENTRANT_UNLOCK(jsReentLock, return JavascriptArray::ReverseHelper(pArr, nullptr, obj, length.GetBigIndex(), scriptContext));
JIT_HELPER_END(Array_Reverse);
@@ -5624,6 +5623,7 @@ using namespace Js;

if (useNoSideEffectReverse)
{
Assert(length <= JavascriptArray::MaxArrayLength);
Recycler * recycler = scriptContext->GetRecycler();

if (length <= 1)
@@ -5774,9 +5774,8 @@ using namespace Js;

failFastOnError.Completed();
}
else if (typedArrayBase)
else if (typedArrayBase && length <= JavascriptArray::MaxArrayLength)
{
Assert(length <= JavascriptArray::MaxArrayLength);
if (typedArrayBase->GetLength() == length)
{
// If typedArrayBase->length == length then we know that the TypedArray will have all items < length
@@ -0,0 +1,60 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");

var tests = [
{
name: "#6179 - Assert in JavascriptArray::EntryReverse when length > Uint32Max for TypedArray with Array prototype",
body: function () {
var ua = new Uint32Array(0x10);
ua.__proto__ = new Array(0xffffffff);
ua.length = 0xffffffff*2;

assert.throws(()=>ua.reverse(), TypeError, "Array#reverse tries to delete a property on the TypedArray but that throws");
}
},
{
name: "#6179 - Assert in JavascriptArray::EntryReverse when length > Uint32Max for TypedArray with own length property",
body: function () {
var ua = new Uint32Array(0x10);
Object.defineProperty(ua, 'length', {value: 0xffffffff*2 });

assert.throws(()=>Array.prototype.reverse.call(ua), TypeError, "Array#reverse tries to delete a property on the TypedArray but that throws");
}
},
{
name: "#6179 - Assert in JavascriptArray::EntryReverse when length > Uint32Max for an object with length property",
body: function () {
let getCount = 0;
let setCount = 0;
var ua = {
length: 0xffffffff*2,
set [0xffffffff*2-1](v) {
assert.areEqual(1, getCount, "1 === getCount");
assert.areEqual(0, setCount, "0 === setCount");
setCount++
},
get '0'() {
assert.areEqual(0, getCount, "0 === getCount");
assert.areEqual(0, setCount, "0 === setCount");
getCount++
},
get '1'() {
assert.areEqual(1, getCount, "1 === getCount");
assert.areEqual(1, setCount, "1 === setCount");
throw 123;
}
};

assert.throws(()=>Array.prototype.reverse.call(ua), 123, "Array#reverse will throw above when we try and get property '1'");

assert.areEqual(1, getCount, "1 === getCount");
assert.areEqual(1, setCount, "1 === setCount");
}
},
];

testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
@@ -602,5 +602,11 @@ Re-enable after fixing ScopeInfo::SaveEnclosingScopeInfo to handle named functio
<files>Bug19948792.js</files>
<compile-flags>-maxinterpretcount:1 -bgjit- -oopjit- -loopinterpretcount:1 -maxsimplejitruncount:2</compile-flags>
</default>
</test>
</test>
<test>
<default>
<files>bug_6179.js</files>
<compile-flags>-args summary -endargs</compile-flags>
</default>
</test>
</regress-exe>

0 comments on commit d404837

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