Fix problems caused by late update of sparse segment's 'left' field

This bug was introduced #2959 (CVE-2017-0238) Revert the move of 'left' field update and add try..catch
chakra-core · May 31, 2017 · 61594a1 · 61594a1
1 parent 6b69d9e
commit 61594a1
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 15 deletions.
diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp
@@ -2078,6 +2078,7 @@ namespace Js
                         limit = JavascriptArray::MaxArrayLength;
                     }
                     seg->size = min(newSize, limit - seg->left);
+                    seg->CheckLengthvsSize();
                 }
             }
             uint32 i;
@@ -7653,6 +7654,8 @@ namespace Js
 
                 Assert(pArr->length <= MaxArrayLength - unshiftElements);
 
+                SparseArraySegmentBase* renumberSeg = pArr->head->next;
+
                 bool isIntArray = false;
                 bool isFloatArray = false;
 
@@ -7683,21 +7686,6 @@ namespace Js
                     }
                 }
 
-                if (isIntArray)
-                {
-                    UnshiftHelper<int32>(pArr, unshiftElements, args.Values);
-                }
-                else if (isFloatArray)
-                {
-                    UnshiftHelper<double>(pArr, unshiftElements, args.Values);
-                }
-                else
-                {
-                    UnshiftHelper<Var>(pArr, unshiftElements, args.Values);
-                }
-
-                SparseArraySegmentBase* renumberSeg = pArr->head->next;
-
                 while (renumberSeg)
                 {
                     renumberSeg->left += unshiftElements;
@@ -7709,6 +7697,26 @@ namespace Js
                     renumberSeg = renumberSeg->next;
                 }
 
+                try
+                {
+                    if (isIntArray)
+                    {
+                        UnshiftHelper<int32>(pArr, unshiftElements, args.Values);
+                    }
+                    else if (isFloatArray)
+                    {
+                        UnshiftHelper<double>(pArr, unshiftElements, args.Values);
+                    }
+                    else
+                    {
+                        UnshiftHelper<Var>(pArr, unshiftElements, args.Values);
+                    }
+                }
+                catch (...)
+                {
+                    Js::Throw::FatalInternalError();
+                }
+
                 pArr->InvalidateLastUsedSegment();
                 pArr->length += unshiftElements;
 

diff --git a/test/Array/bug_12044876.js b/test/Array/bug_12044876.js
@@ -0,0 +1,38 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+//switches: -forcearraybtree
+
+// x86debug: lib\runtime\Library/JavascriptArray.inl, current->left >= lastindex
+function test0() {
+    var arr = [4294967296];
+    arr[9] = 19;
+    arr.unshift(1, 2, {}, 4, 5, 6, 7, 8, 9, 10, 11, 12);
+}
+
+// x64debug: lib\Runtime\Library\SparseArraySegment.cpp, length <= size
+function test1() {
+    function makeArrayLength() {
+      return 100;
+    }
+    var obj0 = {};
+    var protoObj0 = {};
+    var obj1 = {};
+    var arrObj0 = {};
+    var func0 = function () {
+    };
+    var func1 = function () {
+    };
+    obj0.method1 = func0;
+    var ary = Array();
+    var IntArr1 = new Array();
+    IntArr1[15] = ~obj1.prop0;
+    arrObj0.length = makeArrayLength();
+    IntArr1[10] = arrObj0.length;
+    makeArrayLength(IntArr1.unshift(func1(), ary, obj0.method1(), protoObj0, Object(), arrObj0, -1877547837));
+}
+
+test0();
+test1();
+console.log("Pass");
diff --git a/test/Array/rlexe.xml b/test/Array/rlexe.xml
@@ -732,6 +732,13 @@
       <files>bug_9575461.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>bug_12044876.js</files>
+        <compile-flags>-forcearraybtree</compile-flags>
+        <tags>BugFix</tags>
+    </default>
+  </test>
   <test>
     <default>
       <files>array_conv_src.js</files>