Skip to content
Permalink
Browse files

[CVE-2017-0196] Fixing an heap overread during slice.

The MissingItem check is happening on the array in a loop. It is possible that we get called into script and that mutates the array. So the Array's head is newly created with length.
However the loop is still performing over the old length.
Fixed this by checking the length In IsMissingItem function.
Added a unittest.
  • Loading branch information...
akroshg authored and MikeHolman committed Jan 19, 2017
1 parent 05af363 commit 065b7978c40ded35c356ced6cd922a40156c9c46
Showing with 33 additions and 1 deletion.
  1. +6 −1 lib/Runtime/Library/JavascriptArray.cpp
  2. +27 −0 test/Array/Array_TypeConfusion_bugs.js
@@ -477,6 +477,11 @@ namespace Js

bool JavascriptArray::IsMissingItem(uint32 index)
{
if (this->length <= index)
{
return false;
}

bool isIntArray = false, isFloatArray = false;
this->GetArrayTypeAndConvert(&isIntArray, &isFloatArray);

@@ -5767,7 +5772,7 @@ namespace Js
// Prototype lookup for missing elements
if (!pArr->HasNoMissingValues())
{
for (uint32 i = 0; i < newLen; i++)
for (uint32 i = 0; i < newLen && (i + start) < pArr->length; i++)
{
// array type might be changed in the below call to DirectGetItemAtFull
// need recheck array type before checking array item [i + start]
@@ -593,5 +593,32 @@ var tests = [
assert.areEqual(101, arr.length);
}
},
{
name: "Heap overread when splice mutates the array when executing slice",
body: function ()
{
var getterCalled = false;
var a = [1, 2];
for (var i = 0; i < 100 * 1024; i++) {
a.push(i);
}
delete a[0]; // Make a missing item
var protoObj = [11];
Object.defineProperty(protoObj, '0', {
get : function () {
getterCalled = true;
Object.setPrototypeOf(a, Array.prototype);
a.splice(0); // head seg is now length=0
return 42;
},
configurable : true
});
Object.setPrototypeOf(a, protoObj);
var b = a.slice();
assert.isTrue(getterCalled);
assert.areEqual(0, a.length, "Getter will splice the array to zero length");
assert.areEqual(100 * 1024 + 2, b.length, "Validating that slice will return the full array even though splice is deleting the whole array");
}
},
];
testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

0 comments on commit 065b797

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