[CVE-2018-0767] OOB read in AppendLeftOverItemsFromEndSegment - Googl…

…e, Inc.
chakra-core · Jan 5, 2018 · 82825da · 82825da
1 parent 40e45fc
commit 82825da
Showing 1 changed file with 37 additions and 46 deletions.
diff --git a/lib/Runtime/Library/JavascriptArray.inl b/lib/Runtime/Library/JavascriptArray.inl
@@ -695,7 +695,7 @@ SECOND_PASS:
     SparseArraySegment<T>* JavascriptArray::PrepareSegmentForMemOp(uint32 startIndex, uint32 length)
     {
         uint32 endIndex;
-        if(UInt32Math::Add(startIndex, length - 1, &endIndex))
+        if (UInt32Math::Add(startIndex, length - 1, &endIndex))
         {
             return nullptr;
         }
@@ -711,29 +711,38 @@ SECOND_PASS:
             }
         }
 
+        // We are messing with the segments and the length of the array.
+        // We must be certain we reach the end of this function without
+        // any interruption to guaranty coherence of the array
+        AutoDisableInterrupt autoDisableInterrupt(this->GetScriptContext()->GetThreadContext());
+
         this->EnsureHead<T>();
 
         Recycler* recycler = GetRecycler();
 
         //Find the segment where itemIndex is present or is at the boundary
-        SparseArraySegment<T>* current = (SparseArraySegment<T>*)this->GetBeginLookupSegment(startIndex, false);
+        SparseArraySegment<T>* current = (SparseArraySegment<T>*)this->GetLastUsedSegment();
 
         SparseArraySegmentBase* prev = nullptr;
         SparseArraySegmentBase* startSeg = nullptr;
         SparseArraySegmentBase* endSeg = nullptr;
         SparseArraySegmentBase* startPrev = nullptr;
         uint32 growby, startOffset, endOffset;
+        bool isAllocationSolelyInLastUsedSegment = false;
 
-        const auto FindStartAndEndSegment = [&]()
+        // FindStartAndEndSegment
         {
-            if (endIndex >= current->left + current->size)
+            if (current->left > startIndex || endIndex >= current->left + current->size)
             {
+                // The allocation may touch other segments, just start looking from head
                 current = SparseArraySegment<T>::From(head);
             }
             else
             {
+                // We are allocating solely in the last used segment
                 startSeg = endSeg = current;
                 current = nullptr;
+                isAllocationSolelyInLastUsedSegment = true;
             }
 
             while (current != nullptr)
@@ -779,10 +788,12 @@ SECOND_PASS:
             {
                 startPrev = prev;
             }
-        };
+        }
 
-        const auto ResizeArrayIfStartIsOutsideArrayLength = [&]()
+        if (startSeg == nullptr)
         {
+            // if start index is greater than array length then we can add a new segment (or extend the last segment based on some heuristics)
+            // ResizeArrayIfStartIsOutsideArrayLength;
             Assert(endSeg == nullptr);
             Assert(startIndex >= head->size);
             // Reallocate head if it meets a heuristics
@@ -818,10 +829,14 @@ SECOND_PASS:
                     SetHasNoMissingValues();
                 }
             }
-        };
-
-        const auto ExtendStartSegmentForMemOp = [&]()
+        }
+        else
         {
+            // once we found the start segment we extend the start segment until startIndex+length . We don't care about what was there
+            // as they will be overwritten by the memset/ memcopy. Then we need to append items from the (startIndex+length) to array.length
+            // from the end segment to the new array
+
+            // ExtendStartSegmentForMemOp
             SparseArraySegmentBase *oldStartSeg = startSeg;
             bool isInlineSegment = false;
             startOffset = startIndex - startSeg->left;
@@ -874,32 +889,20 @@ SECOND_PASS:
                             }
                         }
                     }
-                    current->length = current->length >  (startOffset + length) ? current->length : (startOffset + length);
+                    current->length = current->length > (startOffset + length) ? current->length : (startOffset + length);
                     current->CheckLengthvsSize();
                     Assert(current == oldStartSeg);
                 }
             }
             else if ((startIndex + 1) <= startSeg->left)
             {
                 isInlineSegment = JavascriptArray::IsInlineSegment(startSeg, this);
-                if (startIndex + 1 == startSeg->left && startPrev == head)
-                {
-                    current = SparseArraySegment<T>::From(head)->GrowByMin(recycler, startIndex + length - head->size);
-                    current->length = endIndex + 1;
-                    current->CheckLengthvsSize();
-                    head = current;
-                }
-                else
+                // startIndex is in between prev and startIndex
+                current = SparseArraySegment<T>::AllocateSegmentImpl<false>(recycler, startIndex, length, nullptr);
+                LinkSegments((Js::SparseArraySegment<T>*)startPrev, current);
+                if (current == head)
                 {
-                    // startIndex is in between prev and startIndex
-                    current = SparseArraySegment<T>::AllocateSegment(recycler, startIndex, length, (SparseArraySegment<T> *)nullptr);
-                    LinkSegments((Js::SparseArraySegment<T>*)startPrev, current);
-                    if (current == head)
-                    {
-                        SetHasNoMissingValues();
-                    }
-                    current->length = length;
-                    current->CheckLengthvsSize();
+                    SetHasNoMissingValues();
                 }
             }
             else
@@ -929,12 +932,11 @@ SECOND_PASS:
             {
                 this->ClearElements(oldStartSeg, 0);
             }
-        };
 
-        const auto AppendLeftOverItemsFromEndSegment = [&]()
-        {
+            // AppendLeftOverItemsFromEndSegment
+
             SparseArraySegmentBase *oldCurrent = current;
-            bool isInlineSegment = false;
+            isInlineSegment = false;
             if (!endSeg)
             {
                 // end is beyond the length of the array
@@ -1018,25 +1020,14 @@ SECOND_PASS:
             {
                 this->ClearElements(oldCurrent, 0);
             }
-        };
-        FindStartAndEndSegment();
-        if (startSeg == nullptr)
-        {
-            // if start index is greater than array length then we can add a new segment (or extend the last segment based on some heuristics)
-            ResizeArrayIfStartIsOutsideArrayLength();
-        }
-        else
-        {
-            // once we found the start segment we extend the start segment until startIndex+length . We don't care about what was there
-            // as they will be overwritten by the memset/ memcopy. Then we need to append items from the (startIndex+length) to array.length
-            // from the end segment to the new array
-            ExtendStartSegmentForMemOp();
-            AppendLeftOverItemsFromEndSegment();
         }
 
         Assert(current);
         Assert(current->left <= startIndex);
         Assert((startIndex - current->left) < current->size);
+        // If we are solely using the last used segment, make sure there were no allocation done
+        Assert(!isAllocationSolelyInLastUsedSegment || (current == startSeg && current == endSeg));
+        autoDisableInterrupt.Completed();
         return current;
     }
 
@@ -1189,7 +1180,7 @@ SECOND_PASS:
         }
         else
         {
-            DirectSetItemAtRangeFull<T>(startIndex, length, newValue);
+            return DirectSetItemAtRangeFull<T>(startIndex, length, newValue);
         }
         return true;
     }