瀏覽代碼

1607 servicing fixes

This change combined fixes for
CVE-2016-3259, CVE-2016-3260, CVE-2016-3265, CVE-2016-3269, CVE-2016-3271
and MS16-085.


MSFT:7558512: [MSRC 33480] Mitigation Bypass Submission - InterpreterThunkEmitter Bypass CFG
Issue.
InterpreterStackFrame class has a member called interpreterThunk which stores the address of our interpreter function's address (regular or the asmjs one). The hacker took advantage of this address 	being stored in the heap memory and corrupted the same to reference a vulnerable shell code.
We do not emit a CFG check for this address before calling, because this is a direct call and not an indirect call.
Fix.
This field is replaced with a boolean - to decide between regular/asmjs interpreter thunk. The address of the interpreter function is obtained in the function which is emitting the code, directly.
This code has been present since the beginning - But this has to serviced only for chakra.dll (till th1), as  we don't have CFG support before that.
Tests.

MSFT:7424216: [MSRC 33319] Chakra Type Confusion JavascriptArray::InternalCopyNativeFloatArrayElements - Individual
[MSRC] Type confusion bug in ChakraCore JavascriptArray::InternalCopyNativeFloatArrayElements.

MSFT:7527933: [MSRC 33383] Chakra JavascriptArray::ForEachOwnMissingArrayIndexOfObject - Individual
[MSRC] Uninitialized stack variable in ChakraCore JavascriptArray::ForEachOwnMissingArrayIndexOfObject Component. Fix by ensuring stack variable was assigned before using.

MSFT:7572196: [MSRC 33354] Edge Chakra ArrayBuffer.transfer - Zero Day Initiative
Fix malloc/realloc usage in ES6 experimental feature ArrayBuffer.transfer. Should zero extra memory in either malloc or realloc case.

MSFT:7387125 7387131 7387136 7387145 7387150 7424221 7424227: [MSRC 33299] Chakra Type Confusion in JavascriptArray::EntryFrom - Individual
[MSRC] Type Confusion in Array built-ins
DirectSetItemAt() is used in numerous Array built-ins without type-checking, where
new objects may be created through a user-defined constructor.
Fix by adding type-checking for type-specialized helper functions, and replacing
DirectSetItemAt() calls with calls to virtual SetItem() functions where applicable.

MSFT:7424474: [MSRC 33332] Edge ReadAV in chakra!Js::JavascriptOperators::StrictEqual+0x18 - Individual
During sort prep we set orig[i] to missing_item. If an exception occurs in the middle (e.g. in "toString"), orig[i] will remain value missing_item, the Array's has_missing_item state could be wrong (wasn't updated), and the Array's content is also corrupted.
Fixed by removing setting orig[i] to missing_item in prep. Do that after sort completion. (It is required to maintain segment length...end to contain only missing_item value.)
Jianchun Xu 9 年之前
父節點
當前提交
17f3d4a485

+ 26 - 12
lib/Backend/InterpreterThunkEmitter.cpp

@@ -189,13 +189,13 @@ const BYTE InterpreterThunkEmitter::HeaderSize = sizeof(InterpreterThunk);
 const BYTE InterpreterThunkEmitter::ThunkSize = sizeof(Call);
 const uint InterpreterThunkEmitter::ThunksPerBlock = (BlockSize - HeaderSize) / ThunkSize;
 
-InterpreterThunkEmitter::InterpreterThunkEmitter(ArenaAllocator* allocator, CustomHeap::CodePageAllocators * codePageAllocators, void * interpreterThunk) :
+InterpreterThunkEmitter::InterpreterThunkEmitter(ArenaAllocator* allocator, CustomHeap::CodePageAllocators * codePageAllocators, bool isAsmInterpreterThunk) :
     emitBufferManager(allocator, codePageAllocators, /*scriptContext*/ nullptr, _u("Interpreter thunk buffer")),
     allocation(nullptr),
     allocator(allocator),
     thunkCount(0),
     thunkBuffer(nullptr),
-    interpreterThunk(interpreterThunk)
+    isAsmInterpreterThunk(isAsmInterpreterThunk)
 {
 }
 
@@ -253,6 +253,20 @@ void InterpreterThunkEmitter::NewThunkBlock()
     DWORD bufferSize = BlockSize;
     DWORD thunkCount = 0;
 
+    void * interpreterThunk = nullptr;
+
+    // the static interpreter thunk invoked by the dynamic emitted thunk
+#ifdef ASMJS_PLAT
+    if (isAsmInterpreterThunk)
+    {
+        interpreterThunk = Js::InterpreterStackFrame::InterpreterAsmThunk;
+    }
+    else
+#endif
+    {
+        interpreterThunk = Js::InterpreterStackFrame::InterpreterThunk;
+    }
+
     allocation = emitBufferManager.AllocateBuffer(bufferSize, &buffer);
     if (!emitBufferManager.ProtectBufferWithExecuteReadWriteForInterpreter(allocation))
     {
@@ -280,7 +294,7 @@ void InterpreterThunkEmitter::NewThunkBlock()
 
     // Copy the thunk buffer and modify it.
     js_memcpy_s(currentBuffer, bytesRemaining, InterpreterThunk, HeaderSize);
-    EncodeInterpreterThunk(currentBuffer, buffer, HeaderSize, epilogStart, epilogSize);
+    EncodeInterpreterThunk(currentBuffer, buffer, HeaderSize, epilogStart, epilogSize, interpreterThunk);
     currentBuffer += HeaderSize;
     bytesRemaining -= HeaderSize;
 
@@ -359,16 +373,16 @@ void InterpreterThunkEmitter::NewThunkBlock()
 
 
 #if _M_ARM
-void InterpreterThunkEmitter::EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE* thunkBuffer, __in_bcount(thunkSize) BYTE* thunkBufferStartAddress, __in const DWORD thunkSize, __in_bcount(epilogSize) BYTE* epilogStart, __in const DWORD epilogSize)
+void InterpreterThunkEmitter::EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE* thunkBuffer, __in_bcount(thunkSize) BYTE* thunkBufferStartAddress, __in const DWORD thunkSize, __in_bcount(epilogSize) BYTE* epilogStart, __in const DWORD epilogSize, __in void * const interpreterThunk)
 {
     _Analysis_assume_(thunkSize == HeaderSize);
     // Encode MOVW
-    DWORD lowerThunkBits = (uint32)this->interpreterThunk & 0x0000FFFF;
+    DWORD lowerThunkBits = (uint32)interpreterThunk & 0x0000FFFF;
     DWORD movW = EncodeMove(/*Opcode*/ 0x0000F240, /*register*/1, lowerThunkBits);
     Emit(thunkBuffer,ThunkAddressOffset, movW);
 
     // Encode MOVT
-    DWORD higherThunkBits = ((uint32)this->interpreterThunk & 0xFFFF0000) >> 16;
+    DWORD higherThunkBits = ((uint32)interpreterThunk & 0xFFFF0000) >> 16;
     DWORD movT = EncodeMove(/*Opcode*/ 0x0000F2C0, /*register*/1, higherThunkBits);
     Emit(thunkBuffer, ThunkAddressOffset + sizeof(movW), movT);
 
@@ -424,7 +438,7 @@ void InterpreterThunkEmitter::GeneratePdata(_In_ const BYTE* entryPoint, _In_ co
 }
 
 #elif _M_ARM64
-void InterpreterThunkEmitter::EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE* thunkBuffer, __in_bcount(thunkSize) BYTE* thunkBufferStartAddress, __in const DWORD thunkSize, __in_bcount(epilogSize) BYTE* epilogStart, __in const DWORD epilogSize)
+void InterpreterThunkEmitter::EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE* thunkBuffer, __in_bcount(thunkSize) BYTE* thunkBufferStartAddress, __in const DWORD thunkSize, __in_bcount(epilogSize) BYTE* epilogStart, __in const DWORD epilogSize, __in void * const interpreterThunk)
 {
     int addrOffset = ThunkAddressOffset;
 
@@ -434,28 +448,28 @@ void InterpreterThunkEmitter::EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE
     // Following 4 MOV Instrs are to move the 64-bit address of the InterpreterThunk address into register x1.
 
     // Encode MOVZ (movz        x1, #<interpreterThunk 16-0 bits>)
-    DWORD lowerThunkBits = (uint64)this->interpreterThunk & 0x0000FFFF;
+    DWORD lowerThunkBits = (uint64)interpreterThunk & 0x0000FFFF;
     DWORD movZ = EncodeMove(/*Opcode*/ 0xD2800000, /*register x1*/1, lowerThunkBits); // no shift; hw = 00
     Emit(thunkBuffer,addrOffset, movZ);
     AssertMsg(sizeof(movZ) == 4, "movZ has to be 32-bit encoded");
     addrOffset+= sizeof(movZ);
 
     // Encode MOVK (movk        x1, #<interpreterThunk 32-16 bits>, lsl #16)
-    DWORD higherThunkBits = ((uint64)this->interpreterThunk & 0xFFFF0000) >> 16;
+    DWORD higherThunkBits = ((uint64)interpreterThunk & 0xFFFF0000) >> 16;
     DWORD movK = EncodeMove(/*Opcode*/ 0xF2A00000, /*register x1*/1, higherThunkBits); // left shift 16 bits; hw = 01
     Emit(thunkBuffer, addrOffset, movK);
     AssertMsg(sizeof(movK) == 4, "movK has to be 32-bit encoded");
     addrOffset+= sizeof(movK);
 
     // Encode MOVK (movk        x1, #<interpreterThunk 48-32 bits>, lsl #16)
-    higherThunkBits = ((uint64)this->interpreterThunk & 0xFFFF00000000) >> 32;
+    higherThunkBits = ((uint64)interpreterThunk & 0xFFFF00000000) >> 32;
     movK = EncodeMove(/*Opcode*/ 0xF2C00000, /*register x1*/1, higherThunkBits); // left shift 32 bits; hw = 02
     Emit(thunkBuffer, addrOffset, movK);
     AssertMsg(sizeof(movK) == 4, "movK has to be 32-bit encoded");
     addrOffset += sizeof(movK);
 
     // Encode MOVK (movk        x1, #<interpreterThunk 64-48 bits>, lsl #16)
-    higherThunkBits = ((uint64)this->interpreterThunk & 0xFFFF000000000000) >> 48;
+    higherThunkBits = ((uint64)interpreterThunk & 0xFFFF000000000000) >> 48;
     movK = EncodeMove(/*Opcode*/ 0xF2E00000, /*register x1*/1, higherThunkBits); // left shift 48 bits; hw = 03
     AssertMsg(sizeof(movK) == 4, "movK has to be 32-bit encoded");
     Emit(thunkBuffer, addrOffset, movK);
@@ -498,7 +512,7 @@ void InterpreterThunkEmitter::GeneratePdata(_In_ const BYTE* entryPoint, _In_ co
     function->FrameSize = 5;                    // the number of bytes of stack that is allocated for this function divided by 16
 }
 #else
-void InterpreterThunkEmitter::EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE* thunkBuffer, __in_bcount(thunkSize) BYTE* thunkBufferStartAddress, __in const DWORD thunkSize, __in_bcount(epilogSize) BYTE* epilogStart, __in const DWORD epilogSize)
+void InterpreterThunkEmitter::EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE* thunkBuffer, __in_bcount(thunkSize) BYTE* thunkBufferStartAddress, __in const DWORD thunkSize, __in_bcount(epilogSize) BYTE* epilogStart, __in const DWORD epilogSize, __in void * const interpreterThunk)
 {
     _Analysis_assume_(thunkSize == HeaderSize);
     Emit(thunkBuffer, ThunkAddressOffset, (uintptr_t)interpreterThunk);

+ 3 - 3
lib/Backend/InterpreterThunkEmitter.h

@@ -61,7 +61,7 @@ private:
     EmitBufferAllocation *allocation;
     SListBase<ThunkBlock> thunkBlocks;
     SListBase<ThunkBlock> freeListedThunkBlocks;
-    void * interpreterThunk; // the static interpreter thunk invoked by the dynamic emitted thunk
+    bool isAsmInterpreterThunk; // To emit address of InterpreterAsmThunk or InterpreterThunk
     BYTE*                thunkBuffer;
     ArenaAllocator*      allocator;
     DWORD thunkCount;                      // Count of thunks available in the current thunk block
@@ -94,7 +94,7 @@ private:
 
     /* ------private helpers -----------*/
     void NewThunkBlock();
-    void EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE* thunkBuffer, __in_bcount(thunkSize) BYTE* thunkBufferStartAddress, __in const DWORD thunkSize, __in_bcount(epilogSize) BYTE* epilogStart, __in const DWORD epilogSize);
+    void EncodeInterpreterThunk(__in_bcount(thunkSize) BYTE* thunkBuffer, __in_bcount(thunkSize) BYTE* thunkBufferStartAddress, __in const DWORD thunkSize, __in_bcount(epilogSize) BYTE* epilogStart, __in const DWORD epilogSize, __in void * const interpreterThunk);
 #if defined(_M_ARM32_OR_ARM64)
     DWORD EncodeMove(DWORD opCode, int reg, DWORD imm16);
     void GeneratePdata(_In_ const BYTE* entryPoint, _In_ const DWORD functionSize, _Out_ RUNTIME_FUNCTION* function);
@@ -116,7 +116,7 @@ public:
     static const uint BlockSize;
     static void* ConvertToEntryPoint(PVOID dynamicInterpreterThunk);
 
-    InterpreterThunkEmitter(ArenaAllocator* allocator, CustomHeap::CodePageAllocators * codePageAllocators, void * interpreterThunk);
+    InterpreterThunkEmitter(ArenaAllocator* allocator, CustomHeap::CodePageAllocators * codePageAllocators, bool isAsmInterpreterThunk = false);
 
     BYTE* GetNextThunk(PVOID* ppDynamicInterpreterThunk);
 

+ 2 - 3
lib/Runtime/Base/ScriptContext.cpp

@@ -1155,13 +1155,12 @@ if (!sourceList)
         }
 
 #if DYNAMIC_INTERPRETER_THUNK
-        interpreterThunkEmitter = HeapNew(InterpreterThunkEmitter, SourceCodeAllocator(), this->GetThreadContext()->GetThunkPageAllocators(), 
-            Js::InterpreterStackFrame::InterpreterThunk);
+        interpreterThunkEmitter = HeapNew(InterpreterThunkEmitter, SourceCodeAllocator(), this->GetThreadContext()->GetThunkPageAllocators());
 #endif
 
 #ifdef ASMJS_PLAT
         asmJsInterpreterThunkEmitter = HeapNew(InterpreterThunkEmitter, SourceCodeAllocator(), this->GetThreadContext()->GetThunkPageAllocators(),
-            Js::InterpreterStackFrame::InterpreterAsmThunk);
+            true);
 #endif
 
         JS_ETW(EtwTrace::LogScriptContextLoadEvent(this));

+ 26 - 3
lib/Runtime/Library/ArrayBuffer.cpp

@@ -878,6 +878,29 @@ namespace Js
 #endif
     }
 
+    // Copy memory from src to dst, truncate if dst smaller, zero extra memory
+    // if dst larger
+    static void MemCpyZero(__bcount(dstSize) BYTE* dst, size_t dstSize,
+                           __in_bcount(count) const BYTE* src, size_t count)
+    {
+        js_memcpy_s(dst, dstSize, src, min(dstSize, count));
+        if (dstSize > count)
+        {
+            ZeroMemory(dst + count, dstSize - count);
+        }
+    }
+
+    // Same as realloc but zero newly allocated portion if newSize > oldSize
+    static BYTE* ReallocZero(BYTE* ptr, size_t oldSize, size_t newSize)
+    {
+        BYTE* ptrNew = (BYTE*)realloc(ptr, newSize);
+        if (ptrNew && newSize > oldSize)
+        {
+            ZeroMemory(ptrNew + oldSize, newSize - oldSize);
+        }
+        return ptrNew;
+    }
+
     ArrayBuffer * JavascriptArrayBuffer::TransferInternal(uint32 newBufferLength)
     {
         ArrayBuffer* newArrayBuffer;
@@ -940,7 +963,7 @@ namespace Js
                         recycler->ReportExternalMemoryFailure(newBufferLength);
                         JavascriptError::ThrowOutOfMemoryError(GetScriptContext());
                     }
-                    js_memcpy_s(newBuffer, newBufferLength, this->buffer, newBufferLength);
+                    MemCpyZero(newBuffer, newBufferLength, this->buffer, this->bufferLength);
                 }
             }
             else
@@ -949,12 +972,12 @@ namespace Js
                 {
                     // we are transferring from an unoptimized buffer, but new length can be optimized, so move to that
                     newBuffer = (BYTE*)JavascriptArrayBuffer::AllocWrapper(newBufferLength);
-                    js_memcpy_s(newBuffer, newBufferLength, this->buffer, newBufferLength);
+                    MemCpyZero(newBuffer, newBufferLength, this->buffer, this->bufferLength);
                 }
                 else if (newBufferLength != this->bufferLength)
                 {
                     // both sides will just be regular ArrayBuffer, so realloc
-                    newBuffer = (BYTE*)realloc(this->buffer, newBufferLength);
+                    newBuffer = ReallocZero(this->buffer, this->bufferLength, newBufferLength);
                     if (!newBuffer)
                     {
                         recycler->ReportExternalMemoryFailure(newBufferLength);

+ 58 - 34
lib/Runtime/Library/JavascriptArray.cpp

@@ -2918,7 +2918,8 @@ namespace Js
                 }
             }
 
-            if (pDestArray && JavascriptArray::IsDirectAccessArray(aItem) && JavascriptArray::IsDirectAccessArray(pDestArray)) // Fast path
+            if (pDestArray && JavascriptArray::IsDirectAccessArray(aItem) && JavascriptArray::IsDirectAccessArray(pDestArray)
+                && BigIndex(idxDest + JavascriptArray::FromVar(aItem)->length).IsSmallIndex()) // Fast path
             {
                 if (JavascriptNativeIntArray::Is(aItem))
                 {
@@ -5700,6 +5701,7 @@ Case0:
         bool isIntArray = false;
         bool isFloatArray = false;
         bool isTypedArrayEntryPoint = typedArrayBase != nullptr;
+        bool isBuiltinArrayCtor = true;
         T startT = 0;
         T newLenT = length;
         T endT = length;
@@ -5752,10 +5754,11 @@ Case0:
         if (isTypedArrayEntryPoint)
         {
             Var constructor = JavascriptOperators::SpeciesConstructor(typedArrayBase, TypedArrayBase::GetDefaultConstructor(args[0], scriptContext), scriptContext);
+            isBuiltinArrayCtor = (constructor == library->GetArrayConstructor());
 
             // If we have an array source object, we need to make sure to do the right thing if it's a native array.
             // The helpers below which do the element copying require the source and destination arrays to have the same native type.
-            if (pArr && constructor == library->GetArrayConstructor())
+            if (pArr && isBuiltinArrayCtor)
             {
                 if (newLenT > JavascriptArray::MaxArrayLength)
                 {
@@ -5789,13 +5792,13 @@ Case0:
 
         else if (pArr != nullptr)
         {
-            newObj = ArraySpeciesCreate(pArr, newLenT, scriptContext, &isIntArray, &isFloatArray);
+            newObj = ArraySpeciesCreate(pArr, newLenT, scriptContext, &isIntArray, &isFloatArray, &isBuiltinArrayCtor);
         }
 
         // skip the typed array and "pure" array case, we still need to handle special arrays like es5array, remote array, and proxy of array.
         else
         {
-            newObj = ArraySpeciesCreate(obj, newLenT, scriptContext);
+            newObj = ArraySpeciesCreate(obj, newLenT, scriptContext, nullptr, nullptr, &isBuiltinArrayCtor);
         }
 
         // If we didn't create a new object above we will create a new array here.
@@ -5839,7 +5842,7 @@ Case0:
         if (pArr)
         {
             // If we constructed a new Array object, we have some nice helpers here
-            if (newArr)
+            if (newArr && isBuiltinArrayCtor)
             {
                 if (JavascriptArray::IsDirectAccessArray(newArr))
                 {
@@ -5885,7 +5888,7 @@ Case0:
                             continue;
                         }
 
-                        newArr->DirectSetItemAt(i, element);
+                        newArr->SetItem(i, element, PropertyOperation_None);
                     }
                 }
             }
@@ -5951,7 +5954,7 @@ Case0:
                     Var element = JavascriptOperators::GetItem(obj, i + start, scriptContext);
                     if (newArr != nullptr)
                     {
-                        newArr->DirectSetItemAt(i, element);
+                        newArr->SetItem(i, element, PropertyOperation_None);
                     }
                     else
                     {
@@ -6238,21 +6241,22 @@ Case0:
                 {
                     countUndefined++;
                 }
-                orig[i] = SparseArraySegment<Var>::GetMissingItem();
             }
         }
 
-        if (count == 0)
+        if (count > 0)
         {
-            *len = 0; // set the length to zero
-            return countUndefined;
-        }
+            SortElements(elements, 0, count - 1);
 
-        SortElements(elements, 0, count - 1);
+            for (uint32 i = 0; i < count; ++i)
+            {
+                orig[i] = elements[i].Value;
+            }
+        }
 
-        for (uint32 i = 0; i < count; ++i)
+        for (uint32 i = count + countUndefined; i < *len; ++i)
         {
-            orig[i] = elements[i].Value;
+            orig[i] = SparseArraySegment<Var>::GetMissingItem();
         }
 
         *len = count; // set the correct length
@@ -6583,6 +6587,7 @@ Case0:
 
             bool isIntArray = false;
             bool isFloatArray = false;
+            bool isBuiltinArrayCtor = true;
             JavascriptArray *newArr = nullptr;
 
             // Just dump the segment map on splice (before any possible allocation and throw)
@@ -6590,7 +6595,7 @@ Case0:
 
             // If the source object is an Array exotic object (Array.isArray) we should try to load the constructor property
             // and use it to construct the return object.
-            newObj = ArraySpeciesCreate(pArr, deleteLen, scriptContext);
+            newObj = ArraySpeciesCreate(pArr, deleteLen, scriptContext, nullptr, nullptr, &isBuiltinArrayCtor);
             if (newObj != nullptr)
             {
                 pArr = EnsureNonNativeArray(pArr);
@@ -6608,7 +6613,7 @@ Case0:
             }
 
             // If return object is a JavascriptArray, we can use all the array splice helpers
-            if (newArr)
+            if (newArr && isBuiltinArrayCtor)
             {
 
                 // Array has a single segment (need not start at 0) and splice start lies in the range
@@ -6687,6 +6692,15 @@ Case0:
                     pArr->length = newLen;
                 }
 
+                if (newArr->length != deleteLen)
+                {
+                    newArr->SetLength(deleteLen);
+                }
+                else
+                {
+                    newArr->length = deleteLen;
+                }
+
                 newArr->InvalidateLastUsedSegment();
 
 #ifdef VALIDATE_ARRAY
@@ -7103,7 +7117,7 @@ Case0:
                    Var element = JavascriptOperators::GetItem(pObj, start + i, scriptContext);
                    if (pnewArr)
                    {
-                       pnewArr->DirectSetItemAt(i, element);
+                       pnewArr->SetItem(i, element, PropertyOperation_None);
                    }
                    else
                    {
@@ -7164,6 +7178,7 @@ Case0:
         // Set up new length
         indexT newLen = indexT(len - deleteLen) + insertLen;
         h.ThrowTypeErrorOnFailure(JavascriptOperators::SetProperty(pObj, pObj, PropertyIds::length, IndexTrace<indexT>::ToNumber(newLen, scriptContext), scriptContext, PropertyOperation_ThrowIfNotExtensible));
+        h.ThrowTypeErrorOnFailure(JavascriptOperators::SetProperty(pNewObj, pNewObj, PropertyIds::length, IndexTrace<indexT>::ToNumber(deleteLen, scriptContext), scriptContext, PropertyOperation_ThrowIfNotExtensible));
 #ifdef VALIDATE_ARRAY
         if (pnewArr)
         {
@@ -8790,6 +8805,7 @@ Case0:
         RecyclableObject* newObj = nullptr;
         JavascriptArray* newArr = nullptr;
         bool isTypedArrayEntryPoint = typedArrayBase != nullptr;
+        bool isBuiltinArrayCtor = true;
 
         if (args.Info.Count < 2 || !JavascriptConversion::IsCallable(args[1]))
         {
@@ -8827,6 +8843,7 @@ Case0:
         {
             Var constructor = JavascriptOperators::SpeciesConstructor(
                 typedArrayBase, TypedArrayBase::GetDefaultConstructor(args[0], scriptContext), scriptContext);
+            isBuiltinArrayCtor = (constructor == scriptContext->GetLibrary()->GetArrayConstructor());
 
             if (JavascriptOperators::IsConstructor(constructor))
             {
@@ -8843,7 +8860,7 @@ Case0:
         // skip the typed array and "pure" array case, we still need to handle special arrays like es5array, remote array, and proxy of array.
         else if (pArr == nullptr || scriptContext->GetConfig()->IsES6SpeciesEnabled())
         {
-            newObj = ArraySpeciesCreate(obj, length, scriptContext);
+            newObj = ArraySpeciesCreate(obj, length, scriptContext, nullptr, nullptr, &isBuiltinArrayCtor);
         }
 
         if (newObj == nullptr)
@@ -8891,7 +8908,7 @@ Case0:
                     pArr);
 
                 // If newArr is a valid pointer, then we constructed an array to return. Otherwise we need to do generic object operations
-                if (newArr)
+                if (newArr && isBuiltinArrayCtor)
                 {
                     newArr->DirectSetItemAt(k, mappedValue);
                 }
@@ -9662,7 +9679,7 @@ Case0:
 
                 if (newArr)
                 {
-                    newArr->DirectSetItemAt(k, nextValue);
+                    newArr->SetItem(k, nextValue, PropertyOperation_None);
                 }
                 else
                 {
@@ -9731,7 +9748,7 @@ Case0:
 
                 if (newArr)
                 {
-                    newArr->DirectSetItemAt(k, kValue);
+                    newArr->SetItem(k, kValue, PropertyOperation_None);
                 }
                 else
                 {
@@ -9781,10 +9798,12 @@ Case0:
         Var newObj = nullptr;
         JavascriptArray* newArr = nullptr;
         TypedArrayBase* newTypedArray = nullptr;
+        bool isBuiltinArrayCtor = true;
 
         if (JavascriptOperators::IsConstructor(args[0]))
         {
             RecyclableObject* constructor = RecyclableObject::FromVar(args[0]);
+            isBuiltinArrayCtor = (constructor == scriptContext->GetLibrary()->GetArrayConstructor());
 
             Js::Var constructorArgs[] = { constructor, JavascriptNumber::ToVar(len, scriptContext) };
             Js::CallInfo constructorCallInfo(Js::CallFlags_New, _countof(constructorArgs));
@@ -9818,7 +9837,7 @@ Case0:
         // At least we have a new object of some kind
         Assert(newObj);
 
-        if (newArr)
+        if (isBuiltinArrayCtor)
         {
             for (uint32 k = 0; k < len; k++)
             {
@@ -9999,11 +10018,11 @@ Case0:
                     if (index < startIndex) continue;
                     else if (index >= limitIndex) break;
 
-                    Var value;
-                    BOOL success = JavascriptOperators::GetOwnItem(es5Array, index, &value, scriptContext);
-                    Assert(success);
-
-                    fn(index, value);
+                    Var value = nullptr;
+                    if (JavascriptOperators::GetOwnItem(es5Array, index, &value, scriptContext))
+                    {
+                        fn(index, value);
+                    }
                 }
             }
         }
@@ -10066,11 +10085,11 @@ Case0:
                         T n = destIndex + (index - startIndex);
                         if (destArray == nullptr || !destArray->DirectGetItemAt(n, &oldValue))
                         {
-                            Var value;
-                            BOOL success = JavascriptOperators::GetOwnItem(es5Array, index, &value, scriptContext);
-                            Assert(success);
-
-                            fn(index, value);
+                            Var value = nullptr;
+                            if (JavascriptOperators::GetOwnItem(es5Array, index, &value, scriptContext))
+                            {
+                                fn(index, value);
+                            }
                         }
                     }
                 }
@@ -11353,7 +11372,7 @@ Case0:
 
     template<typename T>
     RecyclableObject*
-    JavascriptArray::ArraySpeciesCreate(Var originalArray, T length, ScriptContext* scriptContext, bool* pIsIntArray, bool* pIsFloatArray)
+    JavascriptArray::ArraySpeciesCreate(Var originalArray, T length, ScriptContext* scriptContext, bool *pIsIntArray, bool *pIsFloatArray, bool *pIsBuiltinArrayCtor)
     {
         if (originalArray == nullptr || !scriptContext->GetConfig()->IsES6SpeciesEnabled())
         {
@@ -11427,6 +11446,11 @@ Case0:
             JavascriptError::ThrowTypeError(scriptContext, JSERR_NotAConstructor, _u("constructor[Symbol.species]"));
         }
 
+        if (pIsBuiltinArrayCtor != nullptr)
+        {
+            *pIsBuiltinArrayCtor = false;
+        }
+
         Js::Var constructorArgs[] = { constructor, JavascriptNumber::ToVar(length, scriptContext) };
         Js::CallInfo constructorCallInfo(Js::CallFlags_New, _countof(constructorArgs));
 

+ 1 - 1
lib/Runtime/Library/JavascriptArray.h

@@ -767,7 +767,7 @@ namespace Js
         static void ConcatFloatArgs(JavascriptNativeFloatArray* pDestArray, TypeId* remoteTypeIds, Js::Arguments& args, ScriptContext* scriptContext);
     private:
         template<typename T=uint32>
-        static RecyclableObject* ArraySpeciesCreate(Var pThisArray, T length, ScriptContext* scriptContext, bool* pIsIntArray = nullptr, bool* pIsFloatArray = nullptr);
+        static RecyclableObject* ArraySpeciesCreate(Var pThisArray, T length, ScriptContext* scriptContext, bool *pIsIntArray = nullptr, bool *pIsFloatArray = nullptr, bool *pIsBuiltinArrayCtor = nullptr);
         template <typename T, typename R> static R ConvertToIndex(T idxDest, ScriptContext* scriptContext) { Throw::InternalError(); return 0; }
         template <> static Var ConvertToIndex<uint32, Var>(uint32 idxDest, ScriptContext* scriptContext);
         template <> static uint32 ConvertToIndex<uint32, uint32>(uint32 idxDest, ScriptContext* scriptContext) { return idxDest; }

+ 239 - 0
test/Array/Array_TypeConfusion_bugs.js

@@ -0,0 +1,239 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+//Note: see function  ArraySpliceHelper of JavascriptArray.cpp
+
+if (this.WScript && this.WScript.LoadScriptFile) { // Check for running in ch
+    this.WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+}
+
+var tests = [
+    {
+         name: "OS7342663:OOB writes using type confusion in InternalCopyArrayElements",
+         body: function ()
+         {
+             function test() {
+                 var arr1 = [0xdead, 0xbabe, 0xdead, 0xbabe];
+
+                 class MyArray extends Uint32Array { }
+                 Object.defineProperty(MyArray, Symbol.species, { value: function() { return arr1; } });
+
+                 var float_val = 0xdaddeadbabe * 4.9406564584124654E-324;
+                 var test = [float_val, float_val, float_val, float_val];
+                 test.length = 0x1000;
+                 test.__proto__ = new MyArray(0);
+
+                 var res = Array.prototype.slice.apply(test, []);  // OOB write
+                 assert.areEqual(0x1000, res.length, "res.length == 0x1000");
+                 assert.areEqual(float_val, res[0], "res[0] == float_val");
+                 assert.areEqual(float_val, res[1], "res[1] == float_val");
+                 assert.areEqual(float_val, res[2], "res[2] == float_val");
+                 assert.areEqual(float_val, res[3], "res[3] == float_val");
+                 assert.areEqual(undefined, res[4], "res[4] == float_val");
+                 assert.areEqual(undefined, res[0xfff], "res[0xfff] == undefined");
+             }
+             test();
+             test();
+             test();
+         }
+     },
+     {
+         name: "OS7342689:OOB writes using type confusion in InternalFillFromPrototypes",
+         body: function ()
+         {
+             function test() {
+                 var arr1 = [0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead,
+                 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe,
+                 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead,
+                 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe];
+
+                 class MyArray extends Uint32Array { }
+                 Object.defineProperty(MyArray, Symbol.species, { value: function() { return arr1; } });
+
+                 var float_val = 0xdaddeadbabe * 4.9406564584124654E-324;
+                 var test = [{}];
+                 delete test[0];
+                 test.length = 0x1000;
+                 var src = [float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val,
+                 float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val,
+                 float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val,
+                 float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val,
+                 float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val, float_val];
+                 test.__proto__ = src;
+                 test.__proto__.__proto__ = new MyArray(0);
+
+                //this will write 0xfffc0daddeadbabe to [arr1] + 0x1D8
+                 var res = Array.prototype.slice.apply(test, [])
+                 assert.areEqual(0x1000, res.length, "res.length == 0x1000");
+                 assert.areEqual(float_val, res[0], "res[0] == float_val");
+                 assert.areEqual(float_val, res[1], "res[1] == float_val");
+                 assert.areEqual(float_val, res[2], "res[2] == float_val");
+                 assert.areEqual(float_val, res[src.length-1], "res[src.length-1] == float_val");
+                 assert.areEqual(undefined, res[src.length], "res[src] == undefined");
+                 assert.areEqual(undefined, res[0xfff], "res[0xfff] == undefined");
+             }
+             test();
+             test();
+             test();
+         }
+     },
+     {
+         name: "OS7307908:type confusion in Array.prototype.slice",
+         body: function ()
+         {
+             function test() {
+                 var arr = [1, 2]
+
+                //Our species function will get called during chakra!Js::JavascriptArray::SliceHelper<unsigned int>
+                 Object.defineProperty(
+                     arr.constructor,
+                     Symbol.species,
+                     {
+                         value : function()
+                         {
+                            //change 'arr' from TypeIds_NativeIntArray to TypeIds_Array
+                             arr[0] = WScript;
+
+                            //return a TypeIds_NativeIntArray so we can read back out the 64 bit pointer as two 32bit ints.
+                             return [];
+                         }
+                     }
+                 );
+
+                //trigger the bug and retrieve a TypeIds_NativeIntArray array containing a pointer.
+                 var brr = arr.slice();
+
+                 assert.areEqual(2, brr.length, "brr.length == 2");
+                 assert.areEqual(WScript, brr[0], "brr[0] == WScript");
+                 assert.areEqual(2, brr[1], "brr[0] == WScript");
+             }
+             test();
+             test();
+             test();
+         }
+     },
+     {
+         name: "OS7342791:type confusion in Array.from",
+         body: function ()
+         {
+             function test() {
+                 var arr1 = [0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe, 0xdead, 0xbabe];
+
+                 var float_val = 0xdaddeadbabe * 4.9406564584124654E-324;
+                 var test = [float_val, float_val, float_val, float_val];
+                 delete test[0];
+                 delete test[1];
+                 delete test[2];
+
+                 var res = Array.from.apply(function(){return arr1}, [test]);
+                 assert.areEqual(4, res.length, "res.length == 4");
+                 assert.areEqual(undefined, res[0], "res[0] == undefined");
+                 assert.areEqual(undefined, res[1], "res[1] == undefined");
+                 assert.areEqual(undefined, res[2], "res[2] == undefined");
+                 assert.areEqual(float_val, res[3], "res[3] == float_val");
+
+                 assert.areEqual(['1','2','3'], Array.from.apply(()=>new Array(), ["123"]), "Array.from on iterable");
+                 assert.areEqual([1,2,3], Array.from.apply(()=>new Array(), [{"0":1, "1":2, "2":3, "length":3}]), "Array.from on non-iterable");
+             }
+             test();
+             test();
+             test();
+         }
+     },
+     {
+         name: "OS7342844:type confusion in Array.of",
+         body: function ()
+         {
+             function test() {
+                 var brr = Array.of.call(()=>[ 1, 2, 3, 4 ],
+                     WScript, // supply 2 copies of target so the brr array will have a length of 2 and we can read the 64bit pointer.
+                     WScript
+                 );
+
+                 assert.areEqual(2, brr.length, "brr.length == 2");
+                 assert.areEqual(WScript, brr[0], "res[0] == WScript");
+                 assert.areEqual(WScript, brr[1], "res[1] == WScript");
+                 assert.areEqual(undefined, brr[2], "res[2] == undefined");
+                 assert.areEqual(undefined, brr[3], "res[3] == undefined");
+             }
+             test();
+             test();
+             test();
+         }
+     },
+     {
+         name: "OS7342907:type confusion in Array.prototype.map",
+         body: function ()
+         {
+             function test() {
+                 var arr = [ 1, 2 ];
+
+                 Object.defineProperty(
+                     arr.constructor,
+                     Symbol.species,
+                     {
+                         value : function()
+                         {
+                             return [];
+                         }
+                     }
+                 );
+
+                //The value returned from our callback is directly set into the array whose type we create via the species.
+                 var brr = arr.map( function( v )
+                     {
+                         if( v == 1 )
+                             return WScript;
+                     }
+                 );
+
+                 assert.areEqual(2, brr.length, "brr.length == 2");
+                 assert.areEqual(WScript, brr[0], "brr[0] == WScript");
+                 assert.areEqual(undefined, brr[1], "brr[1] == undefined");
+             }
+             test();
+             test();
+             test();
+         }
+     },
+     {
+         name: "OS7342965:type confusion in Array.prototype.splice",
+         body: function ()
+         {
+             function test() {
+                //create a TypeIds_Array holding two 64 bit values (The same amount of space for four 32 bit values).
+                 var arr = [ WScript, WScript ];
+
+                //Our species function will get called during chakra!Js::JavascriptArray::EntrySplice
+                 Object.defineProperty(
+                     arr.constructor,
+                     Symbol.species,
+                     {
+                         value : function()
+                         {
+                            //return a TypeIds_NativeIntArray so we can read back out a 64 bit pointer as two 32bit ints.
+                             return [ 1, 2, 3, 4 ];
+                         }
+                     }
+                 );
+
+                //trigger the bug and retrieve a TypeIds_NativeIntArray array containing a pointer. The helper
+                //method ArraySegmentSpliceHelper<Var> will directly copy over the TypeIds_Array segment data
+                //into the TypeIds_NativeIntArray segment.
+                 var brr = arr.splice( 0, 2 );
+
+                 assert.areEqual(2, brr.length, "brr.length == 2");
+                 assert.areEqual(WScript, brr[0], "brr[0] == WScript");
+                 assert.areEqual(WScript, brr[1], "brr[1] == WScript");
+                 assert.areEqual(undefined, brr[2], "brr[2] == undefined");
+                 assert.areEqual(undefined, brr[3], "brr[3] == undefined");
+             }
+             test();
+             test();
+             test();
+         }
+     },
+];
+testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

+ 20 - 35
test/Array/concat2.baseline

@@ -1,41 +1,26 @@
 -------concat Small-------------
 - concat 101, 102, 103, 104, 105
-length: 2147483653
-  2147483647: 100
-  2147483648: 101
-  2147483649: 102
-  2147483650: 103
-  2147483651: 104
-  2147483652: 105
+length: 2147483647
+  2147483641: 100
+  2147483642: 101
+  2147483643: 102
+  2147483644: 103
+  2147483645: 104
+  2147483646: 105
 - arr.concat(arr)
-length: 4294967295
-  2147483647: 100
-  2147483648: 101
-  2147483649: 102
-  2147483650: 103
-  2147483651: 104
-  2147483652: 105
-  4294967300: 100
-  4294967301: 101
-  4294967302: 102
-  4294967303: 103
-  4294967304: 104
-  4294967305: 105
--------concat Large-------------
-- concat 101, 102, 103, 104, 105
-length: 4294967295
-  4294967293: 100
-  4294967294: 101
-  4294967295: 102
-  4294967296: 103
-  4294967297: 104
-  4294967298: 105
-- arr.concat(arr)
-length: 4294967295
-  4294967293: 100
-  4294967294: 101
-  8589934588: 100
-  8589934589: 101
+length: 4294967294
+  2147483641: 100
+  2147483642: 101
+  2147483643: 102
+  2147483644: 103
+  2147483645: 104
+  2147483646: 105
+  4294967288: 100
+  4294967289: 101
+  4294967290: 102
+  4294967291: 103
+  4294967292: 104
+  4294967293: 105
 -------test prototype lookup-------------
 a: 200,101,202,203,204,105,106,207
 r: 200,101,202,203,204,105,106,207,300,301,302,303,304

+ 5 - 3
test/Array/concat2.js

@@ -38,10 +38,12 @@ function test_concat(size) {
 }
 
 echo("-------concat Small-------------");
-test_concat(2147483648);
+test_concat(2147483642);
 
-echo("-------concat Large-------------");
-test_concat(4294967294);
+// Fix for MSRC 33319 changes concat to skip a fast path if the index we're copying into is a BigIndex.
+// Disable the large portion of this test since this change makes such a test run for hours.
+//echo("-------concat Large-------------");
+//test_concat(4294967294);
 
 echo("-------test prototype lookup-------------");
 for (var i = 0; i < 10; i++) {

+ 7 - 0
test/Array/rlexe.xml

@@ -706,4 +706,11 @@
         <compile-flags>-mic:1 -off:simplejit</compile-flags>
      </default>
   </test>
+  <test>
+    <default>
+      <files>Array_TypeConfusion_bugs.js</files>
+      <compile-flags>-args summary -endargs</compile-flags>
+      <tags>BugFix</tags>
+    </default>
+  </test>
 </regress-exe>

+ 3 - 37
test/es6/es6toLength.js

@@ -12,46 +12,12 @@ var tests = [
        {
             var c = [];
             c[0] = 1;
-            c[4294967294] = 2;
-            var obj = {length : 3, 0 : 3, 1 : 4, 2: 5, [Symbol.isConcatSpreadable] : true}
-            c = c.concat(obj);
-            assert.areEqual(1, c[0], "confirm indices of array concated to did not change")
-            assert.areEqual(2, c[4294967294], "confirm indices of array concated to did not change");
-            assert.areEqual(3, c[4294967295], "confirm obj is spread as properties beyond array length bound");
-            assert.areEqual(4, c[4294967296], "confirm obj is spread as properties beyond array length bound");
-            assert.areEqual(5, c[4294967297], "confirm obj is spread as properties beyond array length bound");
-            assert.areEqual(4294967295, c.length, "length maxes out at 4294967295");
-
-            var c = [];
-            c[0] = 1;
-            c[4294967294] = 2;
-            c = c.concat(3,4,5);
-            assert.areEqual(1, c[0], "confirm indices of array concated to did not change")
-            assert.areEqual(2, c[4294967294], "confirm indices of array concated to did not change");
-            assert.areEqual(3, c[4294967295], "confirm integers are spread as properties beyond array length bound");
-            assert.areEqual(4, c[4294967296], "confirm integers are spread as properties beyond array length bound");
-            assert.areEqual(5, c[4294967297], "confirm integers are spread as properties beyond array length bound");
-            assert.areEqual(4294967295, c.length, "length maxes out at 4294967295");
-
-            var c = [];
-            c[0] = 1;
-            c[4294967294] = 2;
-            c = c.concat([3,4,5]);
-            assert.areEqual(1, c[0], "confirm indices of array concated to did not change")
-            assert.areEqual(2, c[4294967294], "confirm indices of array concated to did not change");
-            assert.areEqual(3, c[4294967295], "confirm array is spread as properties beyond array length bound");
-            assert.areEqual(4, c[4294967296], "confirm array is spread as properties beyond array length bound");
-            assert.areEqual(5, c[4294967297], "confirm array is spread as properties beyond array length bound");
-            assert.areEqual(4294967295, c.length, "length maxes out at 4294967295");
-
-            var c = [];
-            c[0] = 1;
-            c[4294967294] = 2;
+            c[4294967293] = 2;
             var oNeg = { length : -1, 0 : 3, 1: 4, [Symbol.isConcatSpreadable] : true};
             c = c.concat(oNeg);
             assert.areEqual(1, c[0], "confirm indices of array concated to did not change")
-            assert.areEqual(2, c[4294967294], "confirm indices of array concated to did not change");
-            assert.areEqual(undefined, c[4294967295], "Length of oNeg is coerced to 0 nothing is concated here");
+            assert.areEqual(2, c[4294967293], "confirm indices of array concated to did not change");
+            assert.areEqual(undefined, c[4294967294], "Length of oNeg is coerced to 0 nothing is concated here");
        }
    },
    {