[CVE-2018-0993] Edge - Stack-to-heap copy bug in Edge,expose internal property - Qihoo 360

Author: Thomas Moore (CHAKRA)

lib/Runtime/Language/JavascriptOperators.cpp

@@ -9499,9 +9499,9 @@ namespace Js
         case Js::TypeIds_Integer:
             return instance;
         case Js::TypeIds_RegEx:
-            return JavascriptRegExp::BoxStackInstance(JavascriptRegExp::FromVar(instance));
+            return JavascriptRegExp::BoxStackInstance(JavascriptRegExp::FromVar(instance), deepCopy);
         case Js::TypeIds_Object:
-            return DynamicObject::BoxStackInstance(DynamicObject::FromVar(instance));
+            return DynamicObject::BoxStackInstance(DynamicObject::FromVar(instance), deepCopy);
         case Js::TypeIds_Array:
             return JavascriptArray::BoxStackInstance(JavascriptArray::UnsafeFromVar(instance), deepCopy);
         case Js::TypeIds_NativeIntArray:

lib/Runtime/Library/JavascriptArray.cpp

@@ -11668,7 +11668,7 @@ namespace Js
     }
 
     JavascriptArray::JavascriptArray(JavascriptArray * instance, bool boxHead, bool deepCopy)
-        : ArrayObject(instance)
+        : ArrayObject(instance, deepCopy)
     {
         if (boxHead)
         {
@@ -11683,6 +11683,40 @@ namespace Js
         }
     }
 
+    // Allocate a new Array with its own segments and copy the data in instance
+    // into the new Array
+    template <typename T>
+    T * JavascriptArray::DeepCopyInstance(T * instance)
+    {
+        return RecyclerNewPlusZ(instance->GetRecycler(),
+            instance->GetTypeHandler()->GetInlineSlotsSize() + sizeof(Js::SparseArraySegmentBase) + instance->head->size * sizeof(typename T::TElement),
+            T, instance, true /*boxHead*/, true /*deepCopy*/);
+    }
+
+    ArrayObject* JavascriptArray::DeepCopyInstance(ArrayObject* arrayObject)
+    {
+        ArrayObject* arrayCopy;
+        TypeId typeId = JavascriptOperators::GetTypeId(arrayObject);
+        switch (typeId)
+        {
+        case Js::TypeIds_Array:
+            arrayCopy = JavascriptArray::DeepCopyInstance<JavascriptArray>(JavascriptArray::UnsafeFromVar(arrayObject));
+            break;
+        case Js::TypeIds_NativeIntArray:
+            arrayCopy = JavascriptArray::DeepCopyInstance<JavascriptNativeIntArray>(JavascriptNativeIntArray::UnsafeFromVar(arrayObject));
+            break;
+        case Js::TypeIds_NativeFloatArray:
+            arrayCopy = JavascriptArray::DeepCopyInstance<JavascriptNativeFloatArray>(JavascriptNativeFloatArray::UnsafeFromVar(arrayObject));
+            break;
+
+        default:
+            AssertAndFailFast(!"Unexpected objectArray type while boxing stack instance");
+            arrayCopy = nullptr;
+        };
+
+        return arrayCopy;
+    }
+
     template <typename T>
     T * JavascriptArray::BoxStackInstance(T * instance, bool deepCopy)
     {
@@ -11713,9 +11747,16 @@ namespace Js
             // Reallocate both the object as well as the head segment when the head is on the stack or
             // when a deep copy is needed. This is to prevent a scenario where box may leave either one
             // on the stack when both must be on the heap.
-            boxedInstance = RecyclerNewPlusZ(instance->GetRecycler(),
-                inlineSlotsSize + sizeof(Js::SparseArraySegmentBase) + instance->head->size * sizeof(typename T::TElement),
-                T, instance, true, deepCopy);
+            if (deepCopy)
+            {
+                boxedInstance = DeepCopyInstance(instance);
+            }
+            else
+            {
+                boxedInstance = RecyclerNewPlusZ(instance->GetRecycler(),
+                    inlineSlotsSize + sizeof(Js::SparseArraySegmentBase) + instance->head->size * sizeof(typename T::TElement),
+                    T, instance, true /*boxHead*/, false /*deepCopy*/);
+            }
         }
         else if(inlineSlotsSize)
         {
@@ -11803,14 +11844,14 @@ namespace Js
     }
 #endif
 
-    JavascriptNativeArray::JavascriptNativeArray(JavascriptNativeArray * instance) :
-        JavascriptArray(instance, false, false),
+    JavascriptNativeArray::JavascriptNativeArray(JavascriptNativeArray * instance, bool deepCopy) :
+        JavascriptArray(instance, false, deepCopy),
         weakRefToFuncBody(instance->weakRefToFuncBody)
     {
     }
 
     JavascriptNativeIntArray::JavascriptNativeIntArray(JavascriptNativeIntArray * instance, bool boxHead, bool deepCopy) :
-        JavascriptNativeArray(instance)
+        JavascriptNativeArray(instance, deepCopy)
     {
         if (boxHead)
         {
@@ -11856,7 +11897,7 @@ namespace Js
 #endif
 
     JavascriptNativeFloatArray::JavascriptNativeFloatArray(JavascriptNativeFloatArray * instance, bool boxHead, bool deepCopy) :
-        JavascriptNativeArray(instance)
+        JavascriptNativeArray(instance, deepCopy)
     {
         if (boxHead)
         {

lib/Runtime/Library/JavascriptArray.h

@@ -899,10 +899,12 @@ namespace Js
         static uint32 GetSpreadArgLen(Var spreadArg, ScriptContext *scriptContext);
 
         static JavascriptArray * BoxStackInstance(JavascriptArray * instance, bool deepCopy);
+        static ArrayObject * DeepCopyInstance(ArrayObject * instance);
     protected:
         template <typename T> void InitBoxedInlineSegments(SparseArraySegment<T> * dst, SparseArraySegment<T> * src, bool deepCopy);
 
         template <typename T> static T * BoxStackInstance(T * instance, bool deepCopy);
+        template <typename T> static T * DeepCopyInstance(T * instance);
 
     public:
         template<class T, uint InlinePropertySlots> static size_t DetermineAllocationSize(const uint inlineElementSlots, size_t *const allocationPlusSizeRef = nullptr, uint *const alignedInlineElementSlotsRef = nullptr);
@@ -960,7 +962,7 @@ namespace Js
             JavascriptArray(length, type), weakRefToFuncBody(nullptr) {}
 
         // For BoxStackInstance
-        JavascriptNativeArray(JavascriptNativeArray * instance);
+        JavascriptNativeArray(JavascriptNativeArray * instance, bool deepCopy);
 
         Field(RecyclerWeakReference<FunctionBody> *) weakRefToFuncBody;
 

lib/Runtime/Library/JavascriptRegularExpression.cpp

@@ -54,15 +54,20 @@ namespace Js
 #endif
     }
 
-     JavascriptRegExp::JavascriptRegExp(JavascriptRegExp * instance) :
-        DynamicObject(instance),
+     JavascriptRegExp::JavascriptRegExp(JavascriptRegExp * instance, bool deepCopy) :
+        DynamicObject(instance, deepCopy),
         pattern(instance->GetPattern()),
         splitPattern(instance->GetSplitPattern()),
         lastIndexVar(instance->lastIndexVar),
         lastIndexOrFlag(instance->lastIndexOrFlag)
     {
         // For boxing stack instance
         Assert(ThreadContext::IsOnStack(instance));
+
+        // These members should never be on the stack and thus never need to be deep copied
+        Assert(!ThreadContext::IsOnStack(instance->GetPattern()));
+        Assert(!ThreadContext::IsOnStack(instance->GetSplitPattern()));
+        Assert(!ThreadContext::IsOnStack(instance->lastIndexVar));
     }
 
     bool JavascriptRegExp::Is(Var aValue)
@@ -1057,7 +1062,7 @@ namespace Js
     DEFINE_FLAG_GETTER(EntryGetterSticky, sticky, IsSticky)
     DEFINE_FLAG_GETTER(EntryGetterUnicode, unicode, IsUnicode)
 
-    JavascriptRegExp * JavascriptRegExp::BoxStackInstance(JavascriptRegExp * instance)
+    JavascriptRegExp * JavascriptRegExp::BoxStackInstance(JavascriptRegExp * instance, bool deepCopy)
     {
         Assert(ThreadContext::IsOnStack(instance));
         // On the stack, the we reserved a pointer before the object as to store the boxed value
@@ -1068,7 +1073,7 @@ namespace Js
             return boxedInstance;
         }
         Assert(instance->GetTypeHandler()->GetInlineSlotsSize() == 0);
-        boxedInstance = RecyclerNew(instance->GetRecycler(), JavascriptRegExp, instance);
+        boxedInstance = RecyclerNew(instance->GetRecycler(), JavascriptRegExp, instance, deepCopy);
         *boxedInstanceRef = boxedInstance;
         return boxedInstance;
     }

lib/Runtime/Library/JavascriptRegularExpression.h

@@ -82,7 +82,7 @@ namespace Js
         UnifiedRegex::RegexFlags SetRegexFlag(PropertyId propertyId, UnifiedRegex::RegexFlags flags, UnifiedRegex::RegexFlags flag, ScriptContext* scriptContext);
 
         // For boxing stack instance
-        JavascriptRegExp(JavascriptRegExp * instance);
+        JavascriptRegExp(JavascriptRegExp * instance, bool deepCopy);
 
         DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(JavascriptRegExp);
     protected:
@@ -201,7 +201,7 @@ namespace Js
         virtual uint GetSpecialPropertyCount() const override;
         virtual PropertyId const * GetSpecialPropertyIds() const override;
 
-        static Js::JavascriptRegExp * BoxStackInstance(Js::JavascriptRegExp * instance);
+        static Js::JavascriptRegExp * BoxStackInstance(Js::JavascriptRegExp * instance, bool deepCopy);
 
 #if ENABLE_TTD
     public:

lib/Runtime/Types/ArrayObject.cpp

@@ -10,8 +10,8 @@
 
 namespace Js
 {
-    ArrayObject::ArrayObject(ArrayObject * instance)
-        : DynamicObject(instance),
+    ArrayObject::ArrayObject(ArrayObject * instance, bool deepCopy)
+        : DynamicObject(instance, deepCopy),
         length(instance->length)
     {
     }

lib/Runtime/Types/ArrayObject.h

@@ -34,7 +34,7 @@ namespace Js
         }
 
         // For boxing stack instance
-        ArrayObject(ArrayObject * instance);
+        ArrayObject(ArrayObject * instance, bool deepCopy);
 
         void __declspec(noreturn) ThrowItemNotConfigurableError(PropertyId propId = Constants::NoProperty);
         void VerifySetItemAttributes(PropertyId propId, PropertyAttributes attributes);

lib/Runtime/Types/DynamicObject.cpp

@@ -46,7 +46,7 @@ namespace Js
 #endif
     }
 
-    DynamicObject::DynamicObject(DynamicObject * instance) :
+    DynamicObject::DynamicObject(DynamicObject * instance, bool deepCopy) :
         RecyclableObject(instance->type),
         auxSlots(instance->auxSlots),
         objectArray(instance->objectArray)  // copying the array should copy the array flags and array call site index as well
@@ -63,6 +63,8 @@ namespace Js
 #if !FLOATVAR
         ScriptContext * scriptContext = this->GetScriptContext();
 #endif
+        // Copy the inline slot data from the source instance. Deep copy is implicit because
+        // the inline slot allocation is already accounted for with the allocation of the object.
         for (int i = 0; i < inlineSlotCount; i++)
         {
 #if !FLOATVAR
@@ -71,11 +73,27 @@ namespace Js
 #else
             dstSlots[i] = srcSlots[i];
 #endif
-
+            Assert(!ThreadContext::IsOnStack(dstSlots[i]));
         }
 
         if (propertyCount > inlineSlotCapacity)
         {
+            // Properties that are not inlined are stored in the auxSlots, which must be copied
+            // from the source instance.
+
+            // Assert that this block of code will not overwrite inline slot data
+            Assert(!typeHandler->IsObjectHeaderInlinedTypeHandler());
+
+            if (deepCopy)
+            {
+                // When a deepCopy is needed, ensure that auxSlots is not shared with the source instance
+                // so that both objects can have their own, separate lifetimes.
+                InitSlots(this);
+
+                // This auxSlots should now be a separate allocation.
+                Assert(auxSlots != instance->auxSlots);
+            }
+
             uint auxSlotCount = propertyCount - inlineSlotCapacity;
 
             for (uint i = 0; i < auxSlotCount; i++)
@@ -84,11 +102,43 @@ namespace Js
                 // Currently we only support temp numbers assigned to stack objects
                 auxSlots[i] = JavascriptNumber::BoxStackNumber(instance->auxSlots[i], scriptContext);
 #else
+                // Copy the slot values from that instance to this
+                Assert(!ThreadContext::IsOnStack(instance->auxSlots[i]));
                 auxSlots[i] = instance->auxSlots[i];
 #endif
+                Assert(!ThreadContext::IsOnStack(auxSlots[i]));
             }
         }
 
+        if (deepCopy && instance->HasObjectArray())
+        {
+            // Assert that this block of code will not overwrite inline slot data
+            Assert(!typeHandler->IsObjectHeaderInlinedTypeHandler());
+
+            // While the objectArray can be any array type, a DynamicObject that is created on the
+            // stack will only have one of these three types (as these are also the only array types
+            // that can be allocated on the stack).
+            Assert(Js::JavascriptArray::Is(instance->GetObjectArrayOrFlagsAsArray())
+                || Js::JavascriptNativeIntArray::Is(instance->GetObjectArrayOrFlagsAsArray())
+                || Js::JavascriptNativeFloatArray::Is(instance->GetObjectArrayOrFlagsAsArray())
+            );
+
+            // Since a deep copy was requested for this DynamicObject, deep copy the object array as well
+            SetObjectArray(JavascriptArray::DeepCopyInstance(instance->GetObjectArrayOrFlagsAsArray()));
+        }
+        else
+        {
+            // Otherwise, assert that there is either 
+            // - no object array to deep copy
+            // - an object array, but no deep copy needed
+            // - data in the objectArray member, but it is inline slot data
+            // - data in the objectArray member, but it is array flags
+            Assert(
+                (instance->GetObjectArrayOrFlagsAsArray() == nullptr) ||
+                (!deepCopy || typeHandler->IsObjectHeaderInlinedTypeHandler() || instance->UsesObjectArrayOrFlagsAsFlags())
+            );
+        }
+
 #if ENABLE_OBJECT_SOURCE_TRACKING
         TTD::InitializeDiagnosticOriginInformation(this->TTDDiagOriginInfo);
 #endif
@@ -805,7 +855,7 @@ namespace Js
     }
 
     DynamicObject *
-    DynamicObject::BoxStackInstance(DynamicObject * instance)
+    DynamicObject::BoxStackInstance(DynamicObject * instance, bool deepCopy)
     {
         Assert(ThreadContext::IsOnStack(instance));
         // On the stack, the we reserved a pointer before the object as to store the boxed value
@@ -819,11 +869,11 @@ namespace Js
         size_t inlineSlotsSize = instance->GetTypeHandler()->GetInlineSlotsSize();
         if (inlineSlotsSize)
         {
-            boxedInstance = RecyclerNewPlusZ(instance->GetRecycler(), inlineSlotsSize, DynamicObject, instance);
+            boxedInstance = RecyclerNewPlusZ(instance->GetRecycler(), inlineSlotsSize, DynamicObject, instance, deepCopy);
         }
         else
         {
-            boxedInstance = RecyclerNew(instance->GetRecycler(), DynamicObject, instance);
+            boxedInstance = RecyclerNew(instance->GetRecycler(), DynamicObject, instance, deepCopy);
         }
 
         *boxedInstanceRef = boxedInstance;

lib/Runtime/Types/DynamicObject.h

@@ -84,21 +84,40 @@ namespace Js
 #endif
 
     private:
+        // Memory layout of DynamicObject can be one of the following:
+        //        (#1)                (#2)                (#3)
+        //  +--------------+    +--------------+    +--------------+
+        //  | vtable, etc. |    | vtable, etc. |    | vtable, etc. |
+        //  |--------------|    |--------------|    |--------------|
+        //  | auxSlots     |    | auxSlots     |    | inline slots |
+        //  | union        |    | union        |    |              |
+        //  +--------------+    |--------------|    |              |
+        //                      | inline slots |    |              |
+        //                      +--------------+    +--------------+
+        // The allocation size of inline slots is variable and dependent on profile data for the
+        // object. The offset of the inline slots is managed by DynamicTypeHandler.
+        // More details for the layout scenarios below.
+
         Field(Field(Var)*) auxSlots;
-        // The objectArrayOrFlags field can store one of two things:
-        //   a) a pointer to the object array holding numeric properties of this object, or
-        //   b) a bitfield of flags.
+
+        // The objectArrayOrFlags field can store one of three things:
+        //   a) a pointer to the object array holding numeric properties of this object (#1, #2), or
+        //   b) a bitfield of flags (#1, #2), or
+        //   c) inline slot data (#3)
         // Because object arrays are not commonly used, the storage space can be reused to carry information that
         // can improve performance for typical objects. To indicate the bitfield usage we set the least significant bit to 1.
         // Object array pointer always trumps the flags, such that when the first numeric property is added to an
         // object, its flags will be wiped out.  Hence flags can only be used as a form of cache to improve performance.
         // For functional correctness, some other fallback mechanism must exist to convey the information contained in flags.
         // This fields always starts off initialized to null.  Currently, only JavascriptArray overrides it to store flags, the
         // bits it uses are DynamicObjectFlags::AllArrayFlags.
+        // Regarding c) above, inline slots can be stored within the allocation of sizeof(DynamicObject) (#3) or after
+        // sizeof(DynamicObject) (#2). This is indicated by GetTypeHandler()->IsObjectHeaderInlinedTypeHandler(); when true, the
+        // inline slots are within the object, and thus the union members *and* auxSlots actually contain inline slot data.
 
         union
         {
-            Field(ArrayObject *) objectArray;          // Only if !IsAnyArray
+            Field(ArrayObject *) objectArray;       // Only if !IsAnyArray
             struct                                  // Only if IsAnyArray
             {
                 Field(DynamicObjectFlags) arrayFlags;
@@ -122,7 +141,7 @@ namespace Js
         DynamicObject(DynamicType * type, ScriptContext * scriptContext);
 
         // For boxing stack instance
-        DynamicObject(DynamicObject * instance);
+        DynamicObject(DynamicObject * instance, bool deepCopy);
 
         uint16 GetOffsetOfInlineSlots() const;
 
@@ -317,7 +336,7 @@ namespace Js
         ProfileId GetArrayCallSiteIndex() const;
         void SetArrayCallSiteIndex(ProfileId profileId);
 
-        static DynamicObject * BoxStackInstance(DynamicObject * instance);
+        static DynamicObject * BoxStackInstance(DynamicObject * instance, bool deepCopy);
 
     private:
         ArrayObject* EnsureObjectArray();