CVE-2017-0196: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: An information disclosure vulnerability in Microsoft scripting engine allows remote attackers to obtain sensitive information from process memory via a crafted web site, aka "Microsoft Browser Information Disclosure Vulnerability."
CWE description: The product exposes sensitive information to an actor that is not explicitly authorized to have access to that information.
CVSS v3 base score: 6.5
Published: 2017-07-17T13:18Z
Repository: https://github.com/Microsoft/ChakraCore
Language: C++
Files changed: lib/Runtime/Library/JavascriptArray.cpp, test/Array/Array_TypeConfusion_bugs.js
Commit date: 2017-03-16 15:40:15 -0700
Commit message: [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.

Diff: diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp
index fc6d90b36..c344bf8b6 100644
--- a/lib/Runtime/Library/JavascriptArray.cpp
+++ b/lib/Runtime/Library/JavascriptArray.cpp
@@ -475,10 +475,15 @@ namespace Js
         return SparseArraySegment<T>::IsMissingItem(&headSeg->elements[index]);
     }
 
     bool JavascriptArray::IsMissingItem(uint32 index)
     {
+        if (this->length <= index)
+        {
+            return false;
+        }
+
         bool isIntArray = false, isFloatArray = false;
         this->GetArrayTypeAndConvert(&isIntArray, &isFloatArray);
 
         if (isIntArray)
         {
@@ -5765,11 +5770,11 @@ namespace Js
 
         Assert(pnewHeadSeg->length <= pnewHeadSeg->size);
         // 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]
                 if (pArr->IsMissingItem(i + start))
                 {
diff --git a/test/Array/Array_TypeConfusion_bugs.js b/test/Array/Array_TypeConfusion_bugs.js
index e18674249..43430fbc3 100644
--- a/test/Array/Array_TypeConfusion_bugs.js
+++ b/test/Array/Array_TypeConfusion_bugs.js
@@ -591,7 +591,34 @@ var tests = [
             arr.reverse();
             restorePropertyFromDescriptor(Array.prototype, 1, desc);
             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" });

Vulnerable code:             for (uint32 i = 0; i < newLen; i++)
Fixed code:         if (this->length <= index)
        {
            return false;
        }

            for (uint32 i = 0; i < newLen && (i + start) < pArr->length; i++)
    {
        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");
        }
    },