From efab3101028045cbfa0cc21bd852f75bcc037dba Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Tue, 4 Jun 2019 12:19:07 -0700 Subject: [PATCH 1/6] [CVE-2019-1103] Chakra JIT Type Confusion --- lib/Backend/BackwardPass.cpp | 12 ++++++++---- lib/Backend/BailOut.h | 3 ++- lib/Backend/GlobOpt.cpp | 6 ++++++ lib/Backend/Opnd.h | 3 ++- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp index 59210a247b5..de88a05c28f 100644 --- a/lib/Backend/BackwardPass.cpp +++ b/lib/Backend/BackwardPass.cpp @@ -4151,13 +4151,17 @@ BackwardPass::UpdateImplicitCallBailOutKind(IR::Instr *const instr, bool needsBa IR::BailOutKind implicitCallBailOutKind = needsBailOutOnImplicitCall ? IR::BailOutOnImplicitCalls : IR::BailOutInvalid; - const IR::BailOutKind instrBailOutKind = instr->GetBailOutKind(); + IR::BailOutKind instrBailOutKind = instr->GetBailOutKind(); if (instrBailOutKind & IR::BailOutMarkTempObject) { - // Don't remove the implicit call pre op bailout for mark temp object // Remove the mark temp object bit, as we don't need it after the dead store pass - instr->SetBailOutKind(instrBailOutKind & ~IR::BailOutMarkTempObject); - return true; + instrBailOutKind &= ~IR::BailOutMarkTempObject; + instr->SetBailOutKind(instrBailOutKind); + + if (!instr->GetBailOutInfo()->canDeadStore) + { + return true; + } } const IR::BailOutKind instrImplicitCallBailOutKind = instrBailOutKind & ~IR::BailOutKindBits; diff --git a/lib/Backend/BailOut.h b/lib/Backend/BailOut.h index 55af32893b1..208d940552f 100644 --- a/lib/Backend/BailOut.h +++ b/lib/Backend/BailOut.h @@ -27,7 +27,7 @@ class BailOutInfo BailOutInfo(uint32 bailOutOffset, Func* bailOutFunc) : bailOutOffset(bailOutOffset), bailOutFunc(bailOutFunc), byteCodeUpwardExposedUsed(nullptr), polymorphicCacheIndex((uint)-1), startCallCount(0), startCallInfo(nullptr), bailOutInstr(nullptr), - totalOutParamCount(0), argOutSyms(nullptr), bailOutRecord(nullptr), wasCloned(false), isInvertedBranch(false), sharedBailOutKind(true), isLoopTopBailOutInfo(false), + totalOutParamCount(0), argOutSyms(nullptr), bailOutRecord(nullptr), wasCloned(false), isInvertedBranch(false), sharedBailOutKind(true), isLoopTopBailOutInfo(false), canDeadStore(true), outParamInlinedArgSlot(nullptr), liveVarSyms(nullptr), liveLosslessInt32Syms(nullptr), liveFloat64Syms(nullptr), branchConditionOpnd(nullptr), stackLiteralBailOutInfoCount(0), stackLiteralBailOutInfo(nullptr) @@ -69,6 +69,7 @@ class BailOutInfo #endif bool wasCloned; bool isInvertedBranch; + bool canDeadStore; bool sharedBailOutKind; bool isLoopTopBailOutInfo; diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index 652410f7347..d8d1e276c1b 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -16531,6 +16531,7 @@ GlobOpt::GenerateBailOutMarkTempObjectIfNeeded(IR::Instr * instr, IR::Opnd * opn if (instr->HasBailOutInfo()) { instr->SetBailOutKind(instr->GetBailOutKind() | IR::BailOutMarkTempObject); + instr->GetBailOutInfo()->canDeadStore = false; } else { @@ -16540,6 +16541,11 @@ GlobOpt::GenerateBailOutMarkTempObjectIfNeeded(IR::Instr * instr, IR::Opnd * opn || (instr->m_opcode == Js::OpCode::FromVar && !opnd->GetValueType().IsPrimitive()) || propertySymOpnd == nullptr || !propertySymOpnd->IsTypeCheckProtected()) + { + this->GenerateBailAtOperation(&instr, IR::BailOutMarkTempObject); + instr->GetBailOutInfo()->canDeadStore = false; + } + else if (propertySymOpnd->MayHaveImplicitCall()) { this->GenerateBailAtOperation(&instr, IR::BailOutMarkTempObject); } diff --git a/lib/Backend/Opnd.h b/lib/Backend/Opnd.h index 500f5403364..b44ebff5eb3 100644 --- a/lib/Backend/Opnd.h +++ b/lib/Backend/Opnd.h @@ -1138,7 +1138,8 @@ class PropertySymOpnd sealed : public SymOpnd // fall back on live cache. Similarly, for fixed method checks. bool MayHaveImplicitCall() const { - return !IsRootObjectNonConfigurableFieldLoad() && !UsesFixedValue() && (!IsTypeCheckSeqCandidate() || !IsTypeCheckProtected()); + return !IsRootObjectNonConfigurableFieldLoad() && !UsesFixedValue() && (!IsTypeCheckSeqCandidate() || !IsTypeCheckProtected() + || (IsLoadedFromProto() && NeedsWriteGuardTypeCheck())); } // Is the instruction involving this operand part of a type check sequence? This is different from IsObjTypeSpecOptimized From 362e96537af207be3ecf7fa32f338229ee1dcc46 Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Tue, 4 Jun 2019 12:28:16 -0700 Subject: [PATCH 2/6] [CVE-2019-1106] Chakra JIT Overflow --- lib/Backend/Func.cpp | 9 +++++++++ lib/Backend/Func.h | 1 + lib/Backend/GlobOpt.cpp | 11 +++++++++-- lib/Backend/JITOutput.cpp | 6 ++++++ lib/Backend/JITOutput.h | 1 + lib/Backend/NativeCodeGenerator.cpp | 4 ++++ lib/JITIDL/JITTypes.h | 15 ++++++++++----- 7 files changed, 40 insertions(+), 7 deletions(-) diff --git a/lib/Backend/Func.cpp b/lib/Backend/Func.cpp index 9380e19bee6..d866ff21b65 100644 --- a/lib/Backend/Func.cpp +++ b/lib/Backend/Func.cpp @@ -345,6 +345,9 @@ Func::Codegen(JitArenaAllocator *alloc, JITTimeWorkItem * workItem, case RejitReason::TrackIntOverflowDisabled: outputData->disableTrackCompoundedIntOverflow = TRUE; break; + case RejitReason::MemOpDisabled: + outputData->disableMemOp = TRUE; + break; default: Assume(UNREACHED); } @@ -1124,6 +1127,12 @@ Func::IsTrackCompoundedIntOverflowDisabled() const return (HasProfileInfo() && GetReadOnlyProfileInfo()->IsTrackCompoundedIntOverflowDisabled()) || m_output.IsTrackCompoundedIntOverflowDisabled(); } +bool +Func::IsMemOpDisabled() const +{ + return (HasProfileInfo() && GetReadOnlyProfileInfo()->IsMemOpDisabled()) || m_output.IsMemOpDisabled(); +} + bool Func::IsArrayCheckHoistDisabled() const { diff --git a/lib/Backend/Func.h b/lib/Backend/Func.h index 3f7fa575bfc..2a48a97e6ea 100644 --- a/lib/Backend/Func.h +++ b/lib/Backend/Func.h @@ -995,6 +995,7 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece; void SetScopeObjSym(StackSym * sym); StackSym * GetScopeObjSym(); bool IsTrackCompoundedIntOverflowDisabled() const; + bool IsMemOpDisabled() const; bool IsArrayCheckHoistDisabled() const; bool IsStackArgOptDisabled() const; bool IsSwitchOptDisabled() const; diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index d8d1e276c1b..334cac440e6 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -2624,7 +2624,7 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved) !(instr->IsJitProfilingInstr()) && this->currentBlock->loop && !IsLoopPrePass() && !func->IsJitInDebugMode() && - (func->HasProfileInfo() && !func->GetReadOnlyProfileInfo()->IsMemOpDisabled()) && + !func->IsMemOpDisabled() && this->currentBlock->loop->doMemOp) { CollectMemOpInfo(instrPrev, instr, src1Val, src2Val); @@ -16686,7 +16686,14 @@ GlobOpt::GenerateInductionVariableChangeForMemOp(Loop *loop, byte unroll, IR::In } else { - uint size = (loopCount->LoopCountMinusOneConstantValue() + 1) * unroll; + int32 loopCountMinusOnePlusOne; + int32 size; + if (Int32Math::Add(loopCount->LoopCountMinusOneConstantValue(), 1, &loopCountMinusOnePlusOne) || + Int32Math::Mul(loopCountMinusOnePlusOne, unroll, &size)) + { + throw Js::RejitException(RejitReason::MemOpDisabled); + } + Assert(size > 0); sizeOpnd = IR::IntConstOpnd::New(size, IRType::TyUint32, localFunc); } loop->memOpInfo->inductionVariableOpndPerUnrollMap->Add(unroll, sizeOpnd); diff --git a/lib/Backend/JITOutput.cpp b/lib/Backend/JITOutput.cpp index dee344d47f9..2511e9cde72 100644 --- a/lib/Backend/JITOutput.cpp +++ b/lib/Backend/JITOutput.cpp @@ -65,6 +65,12 @@ JITOutput::IsTrackCompoundedIntOverflowDisabled() const return m_outputData->disableTrackCompoundedIntOverflow != FALSE; } +bool +JITOutput::IsMemOpDisabled() const +{ + return m_outputData->disableMemOp != FALSE; +} + bool JITOutput::IsArrayCheckHoistDisabled() const { diff --git a/lib/Backend/JITOutput.h b/lib/Backend/JITOutput.h index 85e9b3c1fa2..10e7a442f11 100644 --- a/lib/Backend/JITOutput.h +++ b/lib/Backend/JITOutput.h @@ -22,6 +22,7 @@ class JITOutput void RecordXData(BYTE * xdata); #endif bool IsTrackCompoundedIntOverflowDisabled() const; + bool IsMemOpDisabled() const; bool IsArrayCheckHoistDisabled() const; bool IsStackArgOptDisabled() const; bool IsSwitchOptDisabled() const; diff --git a/lib/Backend/NativeCodeGenerator.cpp b/lib/Backend/NativeCodeGenerator.cpp index 53b3e746501..8925d809cbe 100644 --- a/lib/Backend/NativeCodeGenerator.cpp +++ b/lib/Backend/NativeCodeGenerator.cpp @@ -1234,6 +1234,10 @@ NativeCodeGenerator::CodeGen(PageAllocator * pageAllocator, CodeGenWorkItem* wor { body->GetAnyDynamicProfileInfo()->DisableTrackCompoundedIntOverflow(); } + if (jitWriteData.disableMemOp) + { + body->GetAnyDynamicProfileInfo()->DisableMemOp(); + } } if (jitWriteData.disableInlineApply) diff --git a/lib/JITIDL/JITTypes.h b/lib/JITIDL/JITTypes.h index 8bc85a78fa5..22ff96db06e 100644 --- a/lib/JITIDL/JITTypes.h +++ b/lib/JITIDL/JITTypes.h @@ -838,37 +838,42 @@ typedef struct JITOutputIDL boolean disableStackArgOpt; boolean disableSwitchOpt; boolean disableTrackCompoundedIntOverflow; - boolean isInPrereservedRegion; + boolean disableMemOp; + boolean isInPrereservedRegion; boolean hasBailoutInstr; - boolean hasJittedStackClosure; + IDL_PAD1(0) unsigned short pdataCount; unsigned short xdataSize; unsigned short argUsedForBranch; + IDL_PAD2(1) int localVarSlotsOffset; // FunctionEntryPointInfo only + int localVarChangedOffset; // FunctionEntryPointInfo only unsigned int frameHeight; - unsigned int codeSize; unsigned int throwMapOffset; + unsigned int throwMapCount; unsigned int inlineeFrameOffsetArrayOffset; - unsigned int inlineeFrameOffsetArrayCount; + unsigned int inlineeFrameOffsetArrayCount; unsigned int propertyGuardCount; + unsigned int ctorCachesCount; + X64_PAD4(2) #if TARGET_64 CHAKRA_PTR xdataAddr; #elif defined(_M_ARM) unsigned int xdataOffset; #else - X86_PAD4(0) + X86_PAD4(3) #endif CHAKRA_PTR codeAddress; CHAKRA_PTR thunkAddress; From d4e767fb946128c135d77edda7a794561e1c1f06 Mon Sep 17 00:00:00 2001 From: Michael Holman Date: Tue, 4 Jun 2019 12:32:09 -0700 Subject: [PATCH 3/6] [CVE-2019-1092] Chakra JIT OOB R/W --- lib/Backend/GlobOptBlockData.cpp | 12 +++++++----- lib/Backend/GlobOptBlockData.h | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/Backend/GlobOptBlockData.cpp b/lib/Backend/GlobOptBlockData.cpp index 31ad9566b1a..46e10460844 100644 --- a/lib/Backend/GlobOptBlockData.cpp +++ b/lib/Backend/GlobOptBlockData.cpp @@ -974,7 +974,8 @@ GlobOptBlockData::MergeValueInfo( fromDataValueInfo->AsArrayValueInfo(), fromDataSym, symsRequiringCompensation, - symsCreatedForMerge); + symsCreatedForMerge, + isLoopBackEdge); } // Consider: If both values are VarConstantValueInfo with the same value, we could @@ -1072,7 +1073,8 @@ ValueInfo *GlobOptBlockData::MergeArrayValueInfo( const ArrayValueInfo *const fromDataValueInfo, Sym *const arraySym, BVSparse *const symsRequiringCompensation, - BVSparse *const symsCreatedForMerge) + BVSparse *const symsCreatedForMerge, + bool isLoopBackEdge) { Assert(mergedValueType.IsAnyOptimizedArray()); Assert(toDataValueInfo); @@ -1095,7 +1097,7 @@ ValueInfo *GlobOptBlockData::MergeArrayValueInfo( } else { - if (!this->globOpt->IsLoopPrePass()) + if (!this->globOpt->IsLoopPrePass() && !isLoopBackEdge) { // Adding compensation code in the prepass won't help, as the symstores would again be different in the main pass. Assert(symsRequiringCompensation); @@ -1123,7 +1125,7 @@ ValueInfo *GlobOptBlockData::MergeArrayValueInfo( } else { - if (!this->globOpt->IsLoopPrePass()) + if (!this->globOpt->IsLoopPrePass() && !isLoopBackEdge) { Assert(symsRequiringCompensation); symsRequiringCompensation->Set(arraySym->m_id); @@ -1150,7 +1152,7 @@ ValueInfo *GlobOptBlockData::MergeArrayValueInfo( } else { - if (!this->globOpt->IsLoopPrePass()) + if (!this->globOpt->IsLoopPrePass() && !isLoopBackEdge) { Assert(symsRequiringCompensation); symsRequiringCompensation->Set(arraySym->m_id); diff --git a/lib/Backend/GlobOptBlockData.h b/lib/Backend/GlobOptBlockData.h index a0fa76dd06f..541c7603411 100644 --- a/lib/Backend/GlobOptBlockData.h +++ b/lib/Backend/GlobOptBlockData.h @@ -264,7 +264,7 @@ class GlobOptBlockData Value * MergeValues(Value *toDataValue, Value *fromDataValue, Sym *fromDataSym, bool isLoopBackEdge, BVSparse *const symsRequiringCompensation, BVSparse *const symsCreatedForMerge); ValueInfo * MergeValueInfo(Value *toDataVal, Value *fromDataVal, Sym *fromDataSym, bool isLoopBackEdge, bool sameValueNumber, BVSparse *const symsRequiringCompensation, BVSparse *const symsCreatedForMerge); JsTypeValueInfo * MergeJsTypeValueInfo(JsTypeValueInfo * toValueInfo, JsTypeValueInfo * fromValueInfo, bool isLoopBackEdge, bool sameValueNumber); - ValueInfo * MergeArrayValueInfo(const ValueType mergedValueType, const ArrayValueInfo *const toDataValueInfo, const ArrayValueInfo *const fromDataValueInfo, Sym *const arraySym, BVSparse *const symsRequiringCompensation, BVSparse *const symsCreatedForMerge); + ValueInfo * MergeArrayValueInfo(const ValueType mergedValueType, const ArrayValueInfo *const toDataValueInfo, const ArrayValueInfo *const fromDataValueInfo, Sym *const arraySym, BVSparse *const symsRequiringCompensation, BVSparse *const symsCreatedForMerge, bool isLoopBackEdge); // Argument Tracking public: From 7f0d390ad77d838cbb81d4586c83ec822f384ce8 Mon Sep 17 00:00:00 2001 From: Paul Leathers Date: Thu, 6 Jun 2019 11:38:26 -0700 Subject: [PATCH 4/6] [CVE-2019-1062] Chakra JIT Type Confusion --- lib/Backend/Opnd.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/Backend/Opnd.cpp b/lib/Backend/Opnd.cpp index d9243e86299..35594cade16 100644 --- a/lib/Backend/Opnd.cpp +++ b/lib/Backend/Opnd.cpp @@ -962,7 +962,8 @@ PropertySymOpnd::IsObjectHeaderInlined() const bool PropertySymOpnd::ChangesObjectLayout() const { - JITTypeHolder cachedType = this->IsMono() ? this->GetType() : this->GetFirstEquivalentType(); + JITTypeHolder cachedType = this->HasInitialType() ? this->GetInitialType() : + this->IsMono() ? this->GetType() : this->GetFirstEquivalentType(); JITTypeHolder finalType = this->GetFinalType(); @@ -987,13 +988,11 @@ PropertySymOpnd::ChangesObjectLayout() const // This is the case where the type transition actually occurs. (This is the only case that's detectable // during the loop pre-pass, since final types are not in place yet.) - Assert(cachedType != nullptr && Js::DynamicType::Is(cachedType->GetTypeId())); - - const JITTypeHandler * cachedTypeHandler = cachedType->GetTypeHandler(); const JITTypeHandler * initialTypeHandler = initialType->GetTypeHandler(); - return cachedTypeHandler->GetInlineSlotCapacity() != initialTypeHandler->GetInlineSlotCapacity() || - cachedTypeHandler->GetOffsetOfInlineSlots() != initialTypeHandler->GetOffsetOfInlineSlots(); + // If no final type has been set in the forward pass, then we have no way of knowing how the object shape will evolve here. + // If the initial type is object-header-inlined, assume that the layout may change. + return initialTypeHandler->IsObjectHeaderInlinedTypeHandler(); } return false; From 214dec9461f9acb9a4b9004368d2a81e0c125652 Mon Sep 17 00:00:00 2001 From: Paul Leathers Date: Thu, 6 Jun 2019 12:58:34 -0700 Subject: [PATCH 5/6] [CVE-2019-1107] Chakra JIT Type Confusion FinishOptPropOp --- lib/Backend/GlobOptFields.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/Backend/GlobOptFields.cpp b/lib/Backend/GlobOptFields.cpp index 92a7e9ec108..59e19b71f6d 100644 --- a/lib/Backend/GlobOptFields.cpp +++ b/lib/Backend/GlobOptFields.cpp @@ -410,6 +410,14 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse *bv, bo if (inGlobOpt) { KillObjectHeaderInlinedTypeSyms(this->currentBlock, false); + if (this->objectTypeSyms) + { + if (this->currentBlock->globOptData.maybeWrittenTypeSyms == nullptr) + { + this->currentBlock->globOptData.maybeWrittenTypeSyms = JitAnew(this->alloc, BVSparse, this->alloc); + } + this->currentBlock->globOptData.maybeWrittenTypeSyms->Or(this->objectTypeSyms); + } } // fall through From 12c31f0e83ddc511e57b9aa1e78533899199eb32 Mon Sep 17 00:00:00 2001 From: Atul Katti Date: Mon, 1 Jul 2019 12:23:53 -0700 Subject: [PATCH 6/6] Update version to 1.11.11 --- 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 f33bbfa17f1..319bf4dceeb 100644 --- a/Build/NuGet/.pack-version +++ b/Build/NuGet/.pack-version @@ -1 +1 @@ -1.11.10 +1.11.11 diff --git a/lib/Common/ChakraCoreVersion.h b/lib/Common/ChakraCoreVersion.h index a13c4fac2fa..4f34d510a40 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 10 +#define CHAKRA_CORE_PATCH_VERSION 11 #define CHAKRA_CORE_VERSION_RELEASE_QFE 0 // Redundant with PATCH_VERSION. Keep this value set to 0. // -------------