From beba75a1aed0933cf3c76efb4dc67529dc035901 Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Wed, 12 Dec 2018 13:32:26 -0800 Subject: [PATCH 01/13] [CVE-2019-0649] Microsoft Chakra JIT server construct Caches array out-of-bounds --- lib/Backend/Func.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Backend/Func.cpp b/lib/Backend/Func.cpp index a99d6ce6a48..9380e19bee6 100644 --- a/lib/Backend/Func.cpp +++ b/lib/Backend/Func.cpp @@ -1673,14 +1673,14 @@ Func::LinkCtorCacheToPropertyId(Js::PropertyId propertyId, JITTimeConstructorCac JITTimeConstructorCache* Func::GetConstructorCache(const Js::ProfileId profiledCallSiteId) { - Assert(profiledCallSiteId < GetJITFunctionBody()->GetProfiledCallSiteCount()); + AssertOrFailFast(profiledCallSiteId < GetJITFunctionBody()->GetProfiledCallSiteCount()); Assert(this->constructorCaches != nullptr); return this->constructorCaches[profiledCallSiteId]; } void Func::SetConstructorCache(const Js::ProfileId profiledCallSiteId, JITTimeConstructorCache* constructorCache) { - Assert(profiledCallSiteId < GetJITFunctionBody()->GetProfiledCallSiteCount()); + AssertOrFailFast(profiledCallSiteId < GetJITFunctionBody()->GetProfiledCallSiteCount()); Assert(constructorCache != nullptr); Assert(this->constructorCaches != nullptr); Assert(this->constructorCaches[profiledCallSiteId] == nullptr); From b778ca81c7d00a8c61b35f6952a9b7b77a2e0a59 Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Wed, 12 Dec 2018 13:33:25 -0800 Subject: [PATCH 02/13] [CVE-2019-0658] Microsoft Chakra JIT server array out-of-bounds access --- lib/Backend/FunctionJITTimeInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Backend/FunctionJITTimeInfo.cpp b/lib/Backend/FunctionJITTimeInfo.cpp index a6b3b409b11..d3287e69ecf 100644 --- a/lib/Backend/FunctionJITTimeInfo.cpp +++ b/lib/Backend/FunctionJITTimeInfo.cpp @@ -302,11 +302,11 @@ FunctionJITTimeInfo::GetRuntimeInfo() const ObjTypeSpecFldInfo * FunctionJITTimeInfo::GetObjTypeSpecFldInfo(uint index) const { - AssertOrFailFast(index < GetBody()->GetInlineCacheCount()); if (m_data.objTypeSpecFldInfoArray == nullptr) { return nullptr; } + AssertOrFailFast(index < m_data.objTypeSpecFldInfoCount); return reinterpret_cast(m_data.objTypeSpecFldInfoArray[index]); } From bdd22d1b6e11738a0a0ad01a884c0bb47cf285d3 Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Wed, 12 Dec 2018 13:37:27 -0800 Subject: [PATCH 03/13] [CVE-2019-0610] Chakra JIT server EnsureLoopBodyLoadSlot out-of-bounds read&write --- lib/Backend/IRBuilder.cpp | 10 +++++++++- lib/Backend/IRBuilderAsmJs.cpp | 6 ++++-- lib/Backend/IRBuilderAsmJs.h | 11 +++++++---- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/Backend/IRBuilder.cpp b/lib/Backend/IRBuilder.cpp index f3edafe3a27..75ee5c7f245 100644 --- a/lib/Backend/IRBuilder.cpp +++ b/lib/Backend/IRBuilder.cpp @@ -774,6 +774,8 @@ IRBuilder::Build() if (!this->RegIsTemp(dstRegSlot) && !this->RegIsConstant(dstRegSlot)) { SymID symId = dstSym->m_id; + + AssertOrFailFast(symId < m_stSlots->Length()); if (this->m_stSlots->Test(symId)) { // For jitted loop bodies that are in a try block, we consider any symbol that has a @@ -4135,7 +4137,12 @@ IRBuilder::EnsureLoopBodyLoadSlot(SymID symId, bool isCatchObjectSym) return; } StackSym * symDst = StackSym::FindOrCreate(symId, (Js::RegSlot)symId, m_func); - if (symDst->m_isCatchObjectSym || this->m_ldSlots->TestAndSet(symId)) + if (symDst->m_isCatchObjectSym) + { + return; + } + AssertOrFailFast(symId < m_ldSlots->Length()); + if (this->m_ldSlots->TestAndSet(symId)) { return; } @@ -4179,6 +4186,7 @@ IRBuilder::SetLoopBodyStSlot(SymID symID, bool isCatchObjectSym) return; } } + AssertOrFailFast(symID < m_stSlots->Length()); this->m_stSlots->Set(symID); } diff --git a/lib/Backend/IRBuilderAsmJs.cpp b/lib/Backend/IRBuilderAsmJs.cpp index efbf8431739..7434dd0b667 100644 --- a/lib/Backend/IRBuilderAsmJs.cpp +++ b/lib/Backend/IRBuilderAsmJs.cpp @@ -3570,7 +3570,10 @@ IRBuilderAsmJs::BuildAsmJsLoopBodySlotOpnd(Js::RegSlot regSlot, IRType opndType) void IRBuilderAsmJs::EnsureLoopBodyAsmJsLoadSlot(Js::RegSlot regSlot, IRType type) { - if (GetJitLoopBodyData().GetLdSlots()->TestAndSet(regSlot)) + BVFixed* ldSlotsBV = GetJitLoopBodyData().GetLdSlots(); + + AssertOrFailFast(regSlot < ldSlotsBV->Length()); + if (ldSlotsBV->TestAndSet(regSlot)) { return; } @@ -3592,7 +3595,6 @@ void IRBuilderAsmJs::EnsureLoopBodyAsmJsStoreSlot(Js::RegSlot regSlot, IRType type) { Assert(!RegIsTemp(regSlot) || RegIsJitLoopYield(regSlot)); - GetJitLoopBodyData().GetStSlots()->Set(regSlot); EnsureLoopBodyAsmJsLoadSlot(regSlot, type); } diff --git a/lib/Backend/IRBuilderAsmJs.h b/lib/Backend/IRBuilderAsmJs.h index ddc24928022..a407eab3f5d 100644 --- a/lib/Backend/IRBuilderAsmJs.h +++ b/lib/Backend/IRBuilderAsmJs.h @@ -26,7 +26,6 @@ struct JitLoopBodyData { private: BVFixed* m_ldSlots = nullptr; - BVFixed* m_stSlots = nullptr; StackSym* m_loopBodyRetIPSym = nullptr; BVFixed* m_yieldRegs = nullptr; uint32 m_loopCurRegs[WAsmJs::LIMIT]; @@ -36,7 +35,6 @@ struct JitLoopBodyData { Assert(ldSlots && stSlots && loopBodyRetIPSym); m_ldSlots = ldSlots; - m_stSlots = stSlots; m_loopBodyRetIPSym = loopBodyRetIPSym; } // Use m_yieldRegs initialization to determine if m_loopCurRegs is initialized @@ -57,14 +55,19 @@ struct JitLoopBodyData } bool IsYieldReg(Js::RegSlot reg) const { - return m_yieldRegs && m_yieldRegs->Test(reg); + if (!m_yieldRegs) + { + return false; + } + AssertOrFailFast(reg < m_yieldRegs->Length()); + return m_yieldRegs->Test(reg); } void SetRegIsYield(Js::RegSlot reg) { Assert(m_yieldRegs); + AssertOrFailFast(reg < m_yieldRegs->Length()); m_yieldRegs->Set(reg); } - BVFixed* GetStSlots() const { return m_stSlots; } BVFixed* GetLdSlots() const { return m_ldSlots; } StackSym* GetLoopBodyRetIPSym() const { return m_loopBodyRetIPSym; } From fc9892ca7eedc869fa8a94c6284156ace3ab4b6c Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Thu, 13 Dec 2018 15:45:03 -0800 Subject: [PATCH 04/13] [CVE-2019-0651] Chakracore Tianfucup IRBuilder::BuildAuxiliary Type Confusion - 360Vulcan --- lib/Runtime/Library/JavascriptArray.cpp | 71 +++++++++++++++---------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp index 4947798d7f0..a762a4d9cbb 100644 --- a/lib/Runtime/Library/JavascriptArray.cpp +++ b/lib/Runtime/Library/JavascriptArray.cpp @@ -982,14 +982,13 @@ using namespace Js; Var JavascriptArray::OP_NewScIntArray(AuxArray *ints, ScriptContext* scriptContext) { JIT_HELPER_NOT_REENTRANT_HEADER(ScrArr_OP_NewScIntArray, reentrancylock, scriptContext->GetThreadContext()); - uint32 count = ints->count; - JavascriptArray *arr = scriptContext->GetLibrary()->CreateArrayLiteral(count); - SparseArraySegment *head = SparseArraySegment::From(arr->head); - Assert(count > 0 && count == head->length); - for (uint i = 0; i < count; i++) - { - head->elements[i] = JavascriptNumber::ToVar(ints->elements[i], scriptContext); - } + + JavascriptNativeIntArray *arr = scriptContext->GetLibrary()->CreateNativeIntArrayLiteral(ints->count); + + SparseArraySegment * segment = (SparseArraySegment*)arr->GetHead(); + + JavascriptOperators::AddIntsToArraySegment(segment, ints); + return arr; JIT_HELPER_END(ScrArr_OP_NewScIntArray); } @@ -1042,7 +1041,16 @@ using namespace Js; return arr; } - return OP_NewScIntArray(ints, scriptContext); + JavascriptArray *arr = scriptContext->GetLibrary()->CreateArrayLiteral(count); + SparseArraySegment *head = SparseArraySegment::From(arr->head); + Assert(count > 0 && count == head->length); + + for (uint i = 0; i < count; i++) + { + head->elements[i] = JavascriptNumber::ToVar(ints->elements[i], scriptContext); + } + + return arr; JIT_HELPER_END(ScrArr_ProfiledNewScIntArray); } #endif @@ -1050,23 +1058,13 @@ using namespace Js; Var JavascriptArray::OP_NewScFltArray(AuxArray *doubles, ScriptContext* scriptContext) { JIT_HELPER_NOT_REENTRANT_HEADER(ScrArr_OP_NewScFltArray, reentrancylock, scriptContext->GetThreadContext()); - uint32 count = doubles->count; - JavascriptArray *arr = scriptContext->GetLibrary()->CreateArrayLiteral(count); - SparseArraySegment *head = SparseArraySegment::From(arr->head); - Assert(count > 0 && count == head->length); - for (uint i = 0; i < count; i++) - { - double dval = doubles->elements[i]; - int32 ival; - if (JavascriptNumber::TryGetInt32Value(dval, &ival) && !TaggedInt::IsOverflow(ival)) - { - head->elements[i] = TaggedInt::ToVarUnchecked(ival); - } - else - { - head->elements[i] = JavascriptNumber::ToVarNoCheck(dval, scriptContext); - } - } + + JavascriptNativeFloatArray *arr = scriptContext->GetLibrary()->CreateNativeFloatArrayLiteral(doubles->count); + + SparseArraySegment * segment = (SparseArraySegment*)arr->GetHead(); + + JavascriptOperators::AddFloatsToArraySegment(segment, doubles); + return arr; JIT_HELPER_END(ScrArr_OP_NewScFltArray); } @@ -1089,7 +1087,26 @@ using namespace Js; return arr; } - return OP_NewScFltArray(doubles, scriptContext); + uint32 count = doubles->count; + JavascriptArray *arr = scriptContext->GetLibrary()->CreateArrayLiteral(count); + SparseArraySegment *head = SparseArraySegment::From(arr->head); + Assert(count > 0 && count == head->length); + + for (uint i = 0; i < count; i++) + { + double dval = doubles->elements[i]; + int32 ival; + if (JavascriptNumber::TryGetInt32Value(dval, &ival) && !TaggedInt::IsOverflow(ival)) + { + head->elements[i] = TaggedInt::ToVarUnchecked(ival); + } + else + { + head->elements[i] = JavascriptNumber::ToVarNoCheck(dval, scriptContext); + } + } + + return arr; JIT_HELPER_END(ScrArr_ProfiledNewScFltArray); } From 5f6dea164251a5cf7aad710a24bbe0c475425563 Mon Sep 17 00:00:00 2001 From: wyrichte Date: Thu, 13 Dec 2018 13:30:41 -0800 Subject: [PATCH 05/13] [CVE-2019-0644] Chakra - AV due to type confusion - Individual - Given a split scope (a function has both a param and body scope), then it is required that the body and param scope are marked as both requiring either a scope object or a scope slot. This was not being enforced in Scope::SetIsObject(). This led to an AV in the interpreter when accessing a property because StLocalSlot was used instead of StLocalObjSlot. --- lib/Runtime/ByteCode/ByteCodeGenerator.cpp | 6 ++++++ lib/Runtime/ByteCode/Scope.cpp | 13 ++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp index 97a2a37e334..aa3b103ce89 100644 --- a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp +++ b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp @@ -2944,6 +2944,12 @@ FuncInfo* PostVisitFunction(ParseNodeFnc* pnodeFnc, ByteCodeGenerator* byteCodeG Scope::MergeParamAndBodyScopes(pnodeFnc); Scope::RemoveParamScope(pnodeFnc); } + else + { + // A param and body scope exist for the same function, they + // should both either be using scope slots or scope objects. + Assert_FailFast(top->bodyScope->GetIsObject() == top->paramScope->GetIsObject()); + } FuncInfo* const parentFunc = byteCodeGenerator->TopFuncInfo(); diff --git a/lib/Runtime/ByteCode/Scope.cpp b/lib/Runtime/ByteCode/Scope.cpp index 0c6c3cd96e4..daef86015a7 100644 --- a/lib/Runtime/ByteCode/Scope.cpp +++ b/lib/Runtime/ByteCode/Scope.cpp @@ -69,11 +69,18 @@ void Scope::SetIsObject() }); } - if (this->GetScopeType() == ScopeType_FunctionBody && funcInfo && !funcInfo->IsBodyAndParamScopeMerged() - && funcInfo->paramScope && !funcInfo->paramScope->GetIsObject()) + // If the scope is split (there exists a body and param scope), then it is required that the + // body and param scope are marked as both requiring either a scope object or a scope slot. + if ((this->GetScopeType() == ScopeType_FunctionBody || this->GetScopeType() == ScopeType_Parameter) + && funcInfo && !funcInfo->IsBodyAndParamScopeMerged()) { - // If this is split scope then mark the param scope also as an object + // The scope is split and one of the scopes (param or body) is being set + // as an object, therefore set both the param and body scopes as objects. + Assert(funcInfo->paramScope); funcInfo->paramScope->SetIsObject(); + + Assert(funcInfo->bodyScope); + funcInfo->bodyScope->SetIsObject(); } } From 15df2a60145d553ceac04e8d1ea60b1519128f4d Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Wed, 19 Dec 2018 00:59:25 -0800 Subject: [PATCH 06/13] [CVE-2019-0640] Bug report for Edge/Chakra: Missing marshalling for Promise result I also added mitigations for bad things that can happen when calling into a closed script context. 1. We delete xdata before unregistering it, which can lead to UAF if we call address of a closed function. Windows Exception code unconditionally jumps to handler address (i.e. without CFG check), so this can bypass CFG. I changed to delete after unregistering. 2. We zero code pages when we close script context, which could be exploitable on x86. I changed to fill with debugbreak. --- lib/Backend/CodeGenWorkItem.cpp | 14 +----- lib/Backend/NativeEntryPointData.cpp | 12 ++--- lib/Backend/NativeEntryPointData.h | 6 +-- lib/Backend/PDataManager.cpp | 3 +- .../Memory/Chakra.Common.Memory.vcxproj | 2 + .../Memory/DelayDeletingFunctionTable.cpp | 18 +++++-- lib/Common/Memory/SectionAllocWrapper.cpp | 48 ++++++++++++++++--- lib/Common/Memory/XDataAllocator.h | 6 +-- lib/Runtime/Library/JavascriptPromise.cpp | 8 ++-- 9 files changed, 72 insertions(+), 45 deletions(-) diff --git a/lib/Backend/CodeGenWorkItem.cpp b/lib/Backend/CodeGenWorkItem.cpp index 44b74803c3c..f5979e6032f 100644 --- a/lib/Backend/CodeGenWorkItem.cpp +++ b/lib/Backend/CodeGenWorkItem.cpp @@ -210,19 +210,7 @@ void CodeGenWorkItem::OnWorkItemProcessFail(NativeCodeGenerator* codeGen) #if PDATA_ENABLED & defined(_WIN32) if (this->entryPointInfo) { - XDataAllocation * xdataAllocation = this->entryPointInfo->GetNativeEntryPointData()->GetXDataInfo(); - if (xdataAllocation) - { - void* functionTable = xdataAllocation->functionTable; - if (functionTable) - { - if (!DelayDeletingFunctionTable::AddEntry(functionTable)) - { - PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("[%d]OnWorkItemProcessFail: Failed to add to slist, table: %llx\n"), GetCurrentThreadId(), functionTable); - DelayDeletingFunctionTable::DeleteFunctionTable(functionTable); - } - } - } + this->entryPointInfo->GetNativeEntryPointData()->CleanupXDataInfo(); } #endif codeGen->FreeNativeCodeGenAllocation(this->allocation->allocation->address); diff --git a/lib/Backend/NativeEntryPointData.cpp b/lib/Backend/NativeEntryPointData.cpp index 79526d672b3..8a77b9c6d29 100644 --- a/lib/Backend/NativeEntryPointData.cpp +++ b/lib/Backend/NativeEntryPointData.cpp @@ -321,20 +321,14 @@ NativeEntryPointData::CleanupXDataInfo() { if (this->xdataInfo != nullptr) { + XDataAllocator::Unregister(this->xdataInfo); #ifdef _WIN32 if (this->xdataInfo->functionTable - && !DelayDeletingFunctionTable::AddEntry(this->xdataInfo->functionTable)) + && !DelayDeletingFunctionTable::AddEntry(this->xdataInfo)) { - DelayDeletingFunctionTable::DeleteFunctionTable(this->xdataInfo->functionTable); + DelayDeletingFunctionTable::DeleteFunctionTable(this->xdataInfo); } #endif - XDataAllocator::Unregister(this->xdataInfo); -#if defined(_M_ARM) - if (JITManager::GetJITManager()->IsOOPJITEnabled()) -#endif - { - HeapDelete(this->xdataInfo); - } this->xdataInfo = nullptr; } } diff --git a/lib/Backend/NativeEntryPointData.h b/lib/Backend/NativeEntryPointData.h index cc4cd8a505d..c9f017a9460 100644 --- a/lib/Backend/NativeEntryPointData.h +++ b/lib/Backend/NativeEntryPointData.h @@ -71,7 +71,8 @@ class NativeEntryPointData #if PDATA_ENABLED XDataAllocation* GetXDataInfo() { return this->xdataInfo; } - void SetXDataInfo(XDataAllocation* xdataInfo) { this->xdataInfo = xdataInfo; } + void CleanupXDataInfo(); + void SetXDataInfo(XDataAllocation* xdataInfo) { this->xdataInfo = xdataInfo; } #endif private: void RegisterEquivalentTypeCaches(Js::ScriptContext * scriptContext, Js::EntryPointInfo * entryPointInfo); @@ -79,9 +80,6 @@ class NativeEntryPointData void FreePropertyGuards(); void FreeNativeCode(Js::ScriptContext * scriptContext, bool isShutdown); -#if PDATA_ENABLED - void CleanupXDataInfo(); -#endif FieldNoBarrier(Js::JavascriptMethod) nativeAddress; FieldNoBarrier(Js::JavascriptMethod) thunkAddress; diff --git a/lib/Backend/PDataManager.cpp b/lib/Backend/PDataManager.cpp index 1b9e44a0d85..8a1514d68db 100644 --- a/lib/Backend/PDataManager.cpp +++ b/lib/Backend/PDataManager.cpp @@ -51,8 +51,7 @@ void PDataManager::UnregisterPdata(RUNTIME_FUNCTION* pdata) { if (AutoSystemInfo::Data.IsWin8OrLater()) { - // TODO: need to move to background? - DelayDeletingFunctionTable::DeleteFunctionTable(pdata); + NtdllLibrary::Instance->DeleteGrowableFunctionTable(pdata); } else { diff --git a/lib/Common/Memory/Chakra.Common.Memory.vcxproj b/lib/Common/Memory/Chakra.Common.Memory.vcxproj index fa72caa747f..77b4552105a 100644 --- a/lib/Common/Memory/Chakra.Common.Memory.vcxproj +++ b/lib/Common/Memory/Chakra.Common.Memory.vcxproj @@ -26,6 +26,8 @@ $(MSBuildThisFileDirectory); + $(MSBuildThisFileDirectory)..\..\JITClient; + $(ChakraJITIDLIntDir); $(MSBuildThisFileDirectory)..; %(AdditionalIncludeDirectories) diff --git a/lib/Common/Memory/DelayDeletingFunctionTable.cpp b/lib/Common/Memory/DelayDeletingFunctionTable.cpp index 1b9b1f38a94..06cdf06e31c 100644 --- a/lib/Common/Memory/DelayDeletingFunctionTable.cpp +++ b/lib/Common/Memory/DelayDeletingFunctionTable.cpp @@ -7,6 +7,7 @@ #include "Memory/XDataAllocator.h" #if PDATA_ENABLED && defined(_WIN32) #include "Core/DelayLoadLibrary.h" +#include "JITClient.h" #include CriticalSection DelayDeletingFunctionTable::cs; @@ -34,14 +35,14 @@ DelayDeletingFunctionTable::~DelayDeletingFunctionTable() } -bool DelayDeletingFunctionTable::AddEntry(FunctionTableHandle ft) +bool DelayDeletingFunctionTable::AddEntry(XDataAllocation* xdata) { if (Head) { FunctionTableNode* node = (FunctionTableNode*)_aligned_malloc(sizeof(FunctionTableNode), MEMORY_ALLOCATION_ALIGNMENT); if (node) { - node->functionTable = ft; + node->xdata = xdata; InterlockedPushEntrySList(Head, &(node->itemEntry)); return true; } @@ -61,7 +62,7 @@ void DelayDeletingFunctionTable::Clear() while (entry) { FunctionTableNode* list = (FunctionTableNode*)entry; - DeleteFunctionTable(list->functionTable); + DeleteFunctionTable(list->xdata); _aligned_free(entry); entry = InterlockedPopEntrySList(Head); } @@ -73,9 +74,16 @@ bool DelayDeletingFunctionTable::IsEmpty() return QueryDepthSList(Head) == 0; } -void DelayDeletingFunctionTable::DeleteFunctionTable(void* functionTable) +void DelayDeletingFunctionTable::DeleteFunctionTable(XDataAllocation* xdata) { - NtdllLibrary::Instance->DeleteGrowableFunctionTable(functionTable); + NtdllLibrary::Instance->DeleteGrowableFunctionTable(xdata->functionTable); + +#if defined(_M_ARM) + if (JITManager::GetJITManager()->IsOOPJITEnabled()) +#endif + { + HeapDelete(xdata); + } } #endif \ No newline at end of file diff --git a/lib/Common/Memory/SectionAllocWrapper.cpp b/lib/Common/Memory/SectionAllocWrapper.cpp index 662fc822275..1205f4c3f3d 100644 --- a/lib/Common/Memory/SectionAllocWrapper.cpp +++ b/lib/Common/Memory/SectionAllocWrapper.cpp @@ -6,7 +6,10 @@ #if _WIN32 #if ENABLE_OOP_NATIVE_CODEGEN -#include "../Core/DelayLoadLibrary.h" +#include "Core/DelayLoadLibrary.h" + +#include "XDataAllocator.h" +#include "CustomHeap.h" #ifdef NTDDI_WIN10_RS2 #if (NTDDI_VERSION >= NTDDI_WIN10_RS2) @@ -603,6 +606,19 @@ SectionAllocWrapper::AllocPages(LPVOID requestAddress, size_t pageCount, DWORD a { return nullptr; } + + // pages could be filled with debugbreak + // zero one page at a time to minimize working set impact while zeroing + for (size_t i = 0; i < dwSize / AutoSystemInfo::PageSize; ++i) + { + LPVOID localAddr = AllocLocal((char*)requestAddress + i * AutoSystemInfo::PageSize, AutoSystemInfo::PageSize); + if (localAddr == nullptr) + { + return nullptr; + } + ZeroMemory(localAddr, AutoSystemInfo::PageSize); + FreeLocal(localAddr); + } address = requestAddress; } @@ -677,10 +693,10 @@ BOOL SectionAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFreeType { return FALSE; } - ZeroMemory(localAddr, AutoSystemInfo::PageSize); + + CustomHeap::FillDebugBreak((BYTE*)localAddr, AutoSystemInfo::PageSize); FreeLocal(localAddr); } - UnlockMemory(this->process, lpAddress, dwSize); } return TRUE; @@ -928,6 +944,19 @@ LPVOID PreReservedSectionAllocWrapper::AllocPages(LPVOID lpAddress, DECLSPEC_GUA addressToReserve = (char*)lpAddress; freeSegmentsBVIndex = (uint)((addressToReserve - (char*)this->preReservedStartAddress) / AutoSystemInfo::Data.GetAllocationGranularityPageSize()); + + // pages could be filled with debugbreak + // zero one page at a time to minimize working set impact while zeroing + for (size_t i = 0; i < dwSize / AutoSystemInfo::PageSize; ++i) + { + LPVOID localAddr = AllocLocal((char*)lpAddress + i * AutoSystemInfo::PageSize, AutoSystemInfo::PageSize); + if (localAddr == nullptr) + { + return nullptr; + } + ZeroMemory(localAddr, AutoSystemInfo::PageSize); + FreeLocal(localAddr); + } #if DBG uint numOfSegments = (uint)ceil((double)dwSize / (double)AutoSystemInfo::Data.GetAllocationGranularityPageSize()); Assert(numOfSegments != 0); @@ -978,12 +1007,17 @@ PreReservedSectionAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFr { return FALSE; } - ZeroMemory(localAddr, AutoSystemInfo::PageSize); + if ((dwFreeType & MEM_RELEASE) == MEM_RELEASE) + { + ZeroMemory(localAddr, AutoSystemInfo::PageSize); + } + else + { + CustomHeap::FillDebugBreak((BYTE*)localAddr, AutoSystemInfo::PageSize); + } FreeLocal(localAddr); } - UnlockMemory(this->process, lpAddress, dwSize); - size_t requestedNumOfSegments = dwSize / AutoSystemInfo::Data.GetAllocationGranularityPageSize(); Assert(requestedNumOfSegments <= MAXUINT32); @@ -1000,6 +1034,8 @@ PreReservedSectionAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFr AssertMsg(freeSegmentsBVIndex < PreReservedAllocationSegmentCount, "Invalid Index ?"); freeSegments.SetRange(freeSegmentsBVIndex, static_cast(requestedNumOfSegments)); PreReservedHeapTrace(_u("MEM_RELEASE: Address: 0x%p of size: 0x%x * 0x%x bytes\n"), lpAddress, requestedNumOfSegments, AutoSystemInfo::Data.GetAllocationGranularityPageSize()); + + UnlockMemory(this->process, lpAddress, dwSize); } return TRUE; diff --git a/lib/Common/Memory/XDataAllocator.h b/lib/Common/Memory/XDataAllocator.h index 9475ccdc875..03c2f1c2d29 100644 --- a/lib/Common/Memory/XDataAllocator.h +++ b/lib/Common/Memory/XDataAllocator.h @@ -16,7 +16,7 @@ struct FunctionTableNode { SLIST_ENTRY itemEntry; - FunctionTableHandle functionTable; + XDataAllocation* xdata; }; struct DelayDeletingFunctionTable @@ -28,9 +28,9 @@ struct DelayDeletingFunctionTable DelayDeletingFunctionTable(); ~DelayDeletingFunctionTable(); - static bool AddEntry(FunctionTableHandle ft); + static bool AddEntry(XDataAllocation* xdata); static void Clear(); static bool IsEmpty(); - static void DeleteFunctionTable(void* functionTable); + static void DeleteFunctionTable(XDataAllocation* xdata); }; #endif \ No newline at end of file diff --git a/lib/Runtime/Library/JavascriptPromise.cpp b/lib/Runtime/Library/JavascriptPromise.cpp index 90a592ef1e0..d916170ceb5 100644 --- a/lib/Runtime/Library/JavascriptPromise.cpp +++ b/lib/Runtime/Library/JavascriptPromise.cpp @@ -1144,15 +1144,17 @@ namespace Js sourcePromise->reactions->Prepend(pair); break; case PromiseStatusCode_HasResolution: - EnqueuePromiseReactionTask(resolveReaction, sourcePromise->result, scriptContext); + EnqueuePromiseReactionTask(resolveReaction, CrossSite::MarshalVar(scriptContext, sourcePromise->result), scriptContext); break; case PromiseStatusCode_HasRejection: + { if (!sourcePromise->GetIsHandled()) { - scriptContext->GetLibrary()->CallNativeHostPromiseRejectionTracker(sourcePromise, sourcePromise->result, true); + scriptContext->GetLibrary()->CallNativeHostPromiseRejectionTracker(sourcePromise, CrossSite::MarshalVar(scriptContext, sourcePromise->result), true); } - EnqueuePromiseReactionTask(rejectReaction, sourcePromise->result, scriptContext); + EnqueuePromiseReactionTask(rejectReaction, CrossSite::MarshalVar(scriptContext, sourcePromise->result), scriptContext); break; + } default: AssertMsg(false, "Promise status is in an invalid state"); break; From 65f1bfee9e4e94d09cd25fe83cb2585f273f945b Mon Sep 17 00:00:00 2001 From: Paul Leathers Date: Fri, 11 Jan 2019 16:43:04 -0800 Subject: [PATCH 07/13] [CVE-2019-0655] [CVE-2019-0642] --- lib/Backend/GlobOptFields.cpp | 46 ++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/lib/Backend/GlobOptFields.cpp b/lib/Backend/GlobOptFields.cpp index 5ea4b6f0b70..cf10627d9a7 100644 --- a/lib/Backend/GlobOptFields.cpp +++ b/lib/Backend/GlobOptFields.cpp @@ -406,7 +406,8 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse *bv, bo KillLiveFields(this->lengthEquivBv, bv); if (inGlobOpt) { - KillObjectHeaderInlinedTypeSyms(this->currentBlock, false); + // Deleting an item, or pushing a property to a non-array, may change object layout + KillAllObjectTypes(bv); } break; @@ -423,27 +424,32 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse *bv, bo case Js::OpCode::CallDirect: fnHelper = instr->GetSrc1()->AsHelperCallOpnd()->m_fnHelper; - // Kill length field for built-ins that can update it. - if(fnHelper == IR::JnHelperMethod::HelperArray_Shift - || fnHelper == IR::JnHelperMethod::HelperArray_Splice - || fnHelper == IR::JnHelperMethod::HelperArray_Unshift) + switch (fnHelper) { - if (nullptr != this->lengthEquivBv) - { - KillLiveFields(this->lengthEquivBv, bv); - } - if (inGlobOpt) - { - KillObjectHeaderInlinedTypeSyms(this->currentBlock, false); - } - } + case IR::JnHelperMethod::HelperArray_Shift: + case IR::JnHelperMethod::HelperArray_Splice: + case IR::JnHelperMethod::HelperArray_Unshift: + // Kill length field for built-ins that can update it. + if (nullptr != this->lengthEquivBv) + { + KillLiveFields(this->lengthEquivBv, bv); + } + // fall through - if ((fnHelper == IR::JnHelperMethod::HelperRegExp_Exec) - || (fnHelper == IR::JnHelperMethod::HelperString_Match) - || (fnHelper == IR::JnHelperMethod::HelperString_Replace)) - { - // Consider: We may not need to kill all fields here. - this->KillAllFields(bv); + case IR::JnHelperMethod::HelperArray_Reverse: + // Deleting an item may change object layout + if (inGlobOpt) + { + KillAllObjectTypes(bv); + } + break; + + case IR::JnHelperMethod::HelperRegExp_Exec: + case IR::JnHelperMethod::HelperString_Match: + case IR::JnHelperMethod::HelperString_Replace: + // Consider: We may not need to kill all fields here. + this->KillAllFields(bv); + break; } break; From 9c4772f9e8d37c79fd3da34ebfd079778ced0fe4 Mon Sep 17 00:00:00 2001 From: Paul Leathers Date: Fri, 11 Jan 2019 17:02:04 -0800 Subject: [PATCH 08/13] [CVE-2019-0590] --- lib/Backend/GlobOpt.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index 3f636e9cf7e..339e599e04e 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -2748,11 +2748,14 @@ GlobOpt::OptTagChecks(IR::Instr *instr) ChangeValueType(nullptr, value, valueType.SetCanBeTaggedValue(false), true /*preserveSubClassInfo*/); return false; } - if (this->byteCodeUses) + if (!this->IsLoopPrePass()) { - this->InsertByteCodeUses(instr); + if (this->byteCodeUses) + { + this->InsertByteCodeUses(instr); + } + this->currentBlock->RemoveInstr(instr); } - this->currentBlock->RemoveInstr(instr); return true; } From 53204ee5b703281ffbb74be52439a30242f64721 Mon Sep 17 00:00:00 2001 From: Paul Leathers Date: Fri, 11 Jan 2019 17:16:19 -0800 Subject: [PATCH 09/13] [CVE-2019-0593] --- lib/Backend/GlobOpt.cpp | 16 +++++++++------- lib/Backend/GlobOpt.h | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index 339e599e04e..1785d98cf24 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -5438,7 +5438,8 @@ GlobOpt::GetPrepassValueTypeForDst( IR::Instr *const instr, Value *const src1Value, Value *const src2Value, - bool const isValueInfoPrecise) const + bool const isValueInfoPrecise, + bool const isSafeToTransferInPrepass) const { // Values with definite types can be created in the loop prepass only when it is guaranteed that the value type will be the // same on any iteration of the loop. The heuristics currently used are: @@ -5455,13 +5456,13 @@ GlobOpt::GetPrepassValueTypeForDst( Assert(IsLoopPrePass()); Assert(instr); - if(!desiredValueType.IsDefinite()) - { - return desiredValueType; - } - if(!isValueInfoPrecise) { + if(!desiredValueType.IsDefinite()) + { + return isSafeToTransferInPrepass ? desiredValueType : desiredValueType.SetCanBeTaggedValue(true); + } + // If the desired value type is not precise, the value type of the destination is derived from the value types of the // sources. Since the value type of a source sym is not definite, the destination value type also cannot be definite. if(desiredValueType.IsInt() && OpCodeAttr::IsInt32(instr->m_opcode)) @@ -5474,6 +5475,7 @@ GlobOpt::GetPrepassValueTypeForDst( // The op always produces a number, but not always an int return desiredValueType.ToDefiniteAnyNumber(); } + // Note: ToLikely() also sets CanBeTaggedValue return desiredValueType.ToLikely(); } @@ -5750,7 +5752,7 @@ GlobOpt::ValueNumberTransferDstInPrepass(IR::Instr *const instr, Value *const sr bool isSafeToTransferInPrepass = false; isValueInfoPrecise = IsPrepassSrcValueInfoPrecise(instr, src1Val, nullptr, &isSafeToTransferInPrepass); - const ValueType valueType(GetPrepassValueTypeForDst(src1ValueInfo->Type(), instr, src1Val, nullptr, isValueInfoPrecise)); + const ValueType valueType(GetPrepassValueTypeForDst(src1ValueInfo->Type(), instr, src1Val, nullptr, isValueInfoPrecise, isSafeToTransferInPrepass)); if(isValueInfoPrecise || isSafeToTransferInPrepass) { Assert(valueType == src1ValueInfo->Type()); diff --git a/lib/Backend/GlobOpt.h b/lib/Backend/GlobOpt.h index 0cdcac4feaf..8fddd809b7d 100644 --- a/lib/Backend/GlobOpt.h +++ b/lib/Backend/GlobOpt.h @@ -561,7 +561,7 @@ class GlobOpt bool AreFromSameBytecodeFunc(IR::RegOpnd const* src1, IR::RegOpnd const* dst) const; Value * ValueNumberDst(IR::Instr **pInstr, Value *src1Val, Value *src2Val); Value * ValueNumberLdElemDst(IR::Instr **pInstr, Value *srcVal); - ValueType GetPrepassValueTypeForDst(const ValueType desiredValueType, IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool const isValueInfoPreciseRef = false) const; + ValueType GetPrepassValueTypeForDst(const ValueType desiredValueType, IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool const isValueInfoPreciseRef = false, bool const isSafeToTransferInPrepass = false) const; bool IsPrepassSrcValueInfoPrecise(IR::Opnd *const src, Value *const srcValue, bool * canTransferValueNumberToDst = nullptr) const; bool IsPrepassSrcValueInfoPrecise(IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool * canTransferValueNumberToDst = nullptr) const; bool IsSafeToTransferInPrepass(StackSym * const sym, ValueInfo *const srcValueInfo) const; From 33c0b9763c231e53e630c0d89d6cb6b0c6139b8e Mon Sep 17 00:00:00 2001 From: Paul Leathers Date: Fri, 11 Jan 2019 18:00:19 -0800 Subject: [PATCH 10/13] [CVE-2019-0605] [CVE-2019-0591] --- lib/Backend/GlobOptFields.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/Backend/GlobOptFields.cpp b/lib/Backend/GlobOptFields.cpp index cf10627d9a7..37fe1ff388f 100644 --- a/lib/Backend/GlobOptFields.cpp +++ b/lib/Backend/GlobOptFields.cpp @@ -237,6 +237,11 @@ GlobOpt::KillLiveElems(IR::IndirOpnd * indirOpnd, BVSparse * this->KillAllFields(bv); // This also kills all property type values, as the same bit-vector tracks those stack syms SetAnyPropertyMayBeWrittenTo(); } + else if (inGlobOpt && indexOpnd && !indexOpnd->GetValueType().IsInt() && !currentBlock->globOptData.IsInt32TypeSpecialized(indexOpnd->m_sym)) + { + // Write/delete to a non-integer numeric index can't alias a name on the RHS of a dot, but it change object layout + this->KillAllObjectTypes(bv); + } } void From 1a7790f873b1a73d1cfec9548eb08a3b9fd798f3 Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Tue, 22 Jan 2019 15:14:37 -0800 Subject: [PATCH 11/13] [CVE-2019-0648] Edge - ChakraCore OOB read - Individual --- lib/Parser/RegexParser.cpp | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/lib/Parser/RegexParser.cpp b/lib/Parser/RegexParser.cpp index 1db3ecc6849..b19978c24c7 100644 --- a/lib/Parser/RegexParser.cpp +++ b/lib/Parser/RegexParser.cpp @@ -2493,32 +2493,23 @@ namespace UnifiedRegex case 'W': return false; case 'c': - if (standardEncodedChars->IsLetter(ECLookahead())) // terminating 0 is not a letter + if (!standardEncodedChars->IsLetter(ECLookahead())) //Letter set [A-Z, a-z] + { + // Fail in unicode mode for non-letter escaped control characters according to 262 Annex-B RegExp grammar spec #prod-annexB-Term + DeferredFailIfUnicode(JSERR_RegExpInvalidEscape); + } + + if (standardEncodedChars->IsWord(ECLookahead())) // word set [A-Z,a-z,0-9,_], terminating 0 is not a word character { singleton = UTC(Chars::CTU(ECLookahead()) % 32); ECConsume(); } else { - DeferredFailIfUnicode(JSERR_RegExpInvalidEscape); // Fail in unicode mode for non-letter escaped control characters according to 262 Annex-B RegExp grammar spec #prod-annexB-Term - - if (!IsEOF()) - { - EncodedChar ecLookahead = ECLookahead(); - switch (ecLookahead) - { - case '-': - case ']': - singleton = c; - break; - default: - singleton = UTC(Chars::CTU(ecLookahead) % 32); - ECConsume(); - break; - } - } - else - singleton = c; + // If the lookahead is a non-alphanumeric and not an underscore ('_'), then treat '\' and 'c' separately. + //#sec-regular-expression-patterns-semantics + ECRevert(1); //Put cursor back at 'c' and treat it as a non-escaped character. + singleton = '\\'; } return true; case 'x': From 61e13a619fa5c172fa57e5e9e6306189f45ae839 Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Wed, 23 Jan 2019 12:27:39 -0800 Subject: [PATCH 12/13] [CVE-2019-0607] WebAssembly - pointer value control issue --- lib/Runtime/Library/WebAssemblyInstance.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Runtime/Library/WebAssemblyInstance.cpp b/lib/Runtime/Library/WebAssemblyInstance.cpp index 8db7c14a210..bfb8660adb0 100644 --- a/lib/Runtime/Library/WebAssemblyInstance.cpp +++ b/lib/Runtime/Library/WebAssemblyInstance.cpp @@ -268,10 +268,10 @@ Var WebAssemblyInstance::CreateExportObject(WebAssemblyModule * wasmModule, Scri case Wasm::WasmTypes::I64: JavascriptError::ThrowTypeErrorVar(wasmModule->GetScriptContext(), WASMERR_InvalidTypeConversion, _u("i64"), _u("Var")); case Wasm::WasmTypes::F32: - obj = JavascriptNumber::New(cnst.f32, scriptContext); + obj = JavascriptNumber::NewWithCheck(cnst.f32, scriptContext); break; case Wasm::WasmTypes::F64: - obj = JavascriptNumber::New(cnst.f64, scriptContext); + obj = JavascriptNumber::NewWithCheck(cnst.f64, scriptContext); break; #ifdef ENABLE_WASM_SIMD case Wasm::WasmTypes::M128: From a54c9cb92464388a396be64435a80cfcb729a545 Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Thu, 24 Jan 2019 14:21:46 -0800 Subject: [PATCH 13/13] update version number --- Build/NuGet/.pack-version | 2 +- lib/Common/ChakraCoreVersion.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Build/NuGet/.pack-version b/Build/NuGet/.pack-version index e6dbb7c238f..6b37cb71cf8 100644 --- a/Build/NuGet/.pack-version +++ b/Build/NuGet/.pack-version @@ -1 +1 @@ -1.11.5 +1.11.6 diff --git a/lib/Common/ChakraCoreVersion.h b/lib/Common/ChakraCoreVersion.h index e65afe91db0..abd0c464d37 100644 --- a/lib/Common/ChakraCoreVersion.h +++ b/lib/Common/ChakraCoreVersion.h @@ -17,7 +17,7 @@ // ChakraCore version number definitions (used in ChakraCore binary metadata) #define CHAKRA_CORE_MAJOR_VERSION 1 #define CHAKRA_CORE_MINOR_VERSION 11 -#define CHAKRA_CORE_PATCH_VERSION 5 +#define CHAKRA_CORE_PATCH_VERSION 6 #define CHAKRA_CORE_VERSION_RELEASE_QFE 0 // Redundant with PATCH_VERSION. Keep this value set to 0. // -------------