From 8d21cde342a4e826d96eb4ac85241f876b879aed Mon Sep 17 00:00:00 2001 From: Akrosh Gandhi Date: Wed, 14 Nov 2018 11:46:00 -0800 Subject: [PATCH 1/5] CVE-2018-8583 Edge - Chakra JIT OOB 9 13 leads to RCE In the loop range check we emit add instruction to add 1 to the range. That can overflow. We did't have overflow bailout over there. Fixed that by adding bailout over there. --- lib/Backend/GlobOptIntBounds.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/Backend/GlobOptIntBounds.cpp b/lib/Backend/GlobOptIntBounds.cpp index 58cd0148af7..5136013b1dc 100644 --- a/lib/Backend/GlobOptIntBounds.cpp +++ b/lib/Backend/GlobOptIntBounds.cpp @@ -1822,11 +1822,16 @@ void GlobOpt::GenerateLoopCountPlusOne(Loop *const loop, LoopCount *const loopCo IR::RegOpnd *loopCountOpnd = IR::RegOpnd::New(type, func); IR::RegOpnd *minusOneOpnd = IR::RegOpnd::New(loopCount->LoopCountMinusOneSym(), type, func); minusOneOpnd->SetIsJITOptimizedReg(true); - insertBeforeInstr->InsertBefore(IR::Instr::New(Js::OpCode::Add_I4, - loopCountOpnd, - minusOneOpnd, - IR::IntConstOpnd::New(1, type, func, true), - func)); + IR::Instr* incrInstr = IR::Instr::New(Js::OpCode::Add_I4, + loopCountOpnd, + minusOneOpnd, + IR::IntConstOpnd::New(1, type, func, true), + func); + + insertBeforeInstr->InsertBefore(incrInstr); + + // Incrementing to 1 can overflow - add a bounds check bailout here + incrInstr->ConvertToBailOutInstr(bailOutInfo, IR::BailOutOnFailedHoistedLoopCountBasedBoundCheck); loopCount->SetLoopCountSym(loopCountOpnd->GetStackSym()); } } From 8264b9bcdb08daf4309415319c7a8e03d1736dce Mon Sep 17 00:00:00 2001 From: Wyatt Richter Date: Wed, 14 Nov 2018 12:30:21 -0800 Subject: [PATCH 2/5] CVE-2018-8624 Edge - Chakra JIT Overflow --- lib/Backend/BackwardPass.cpp | 10 +++++++++- lib/Backend/FlowGraph.h | 1 + lib/Backend/GlobOpt.cpp | 3 +++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp index cdc79c88379..d94e79fab7f 100644 --- a/lib/Backend/BackwardPass.cpp +++ b/lib/Backend/BackwardPass.cpp @@ -8669,7 +8669,15 @@ BackwardPass::RestoreInductionVariableValuesAfterMemOp(Loop *loop) IR::Opnd *inductionVariableOpnd = IR::RegOpnd::New(sym, IRType::TyInt32, localFunc); IR::Opnd *sizeOpnd = globOpt->GenerateInductionVariableChangeForMemOp(loop, inductionVariableChangeInfo.unroll); - loop->landingPad->InsertAfter(IR::Instr::New(opCode, inductionVariableOpnd, inductionVariableOpnd, sizeOpnd, loop->GetFunc())); + IR::Instr* restoreInductionVarInstr = IR::Instr::New(opCode, inductionVariableOpnd, inductionVariableOpnd, sizeOpnd, loop->GetFunc()); + + // The IR that restores the induction variable's value is placed before the MemOp. Since this IR can + // bailout to the loop's landing pad, placing this IR before the MemOp avoids performing the MemOp, + // bailing out because of this IR, and then performing the effects of the loop again. + loop->landingPad->InsertInstrBefore(restoreInductionVarInstr, loop->memOpInfo->instr); + + // If restoring an induction variable results in an overflow, bailout to the loop's landing pad. + restoreInductionVarInstr->ConvertToBailOutInstr(loop->bailOutInfo, IR::BailOutOnOverflow); }; for (auto it = loop->memOpInfo->inductionVariableChangeInfoMap->GetIterator(); it.IsValid(); it.MoveNext()) diff --git a/lib/Backend/FlowGraph.h b/lib/Backend/FlowGraph.h index 24c8e61798a..bcfbca7c15e 100644 --- a/lib/Backend/FlowGraph.h +++ b/lib/Backend/FlowGraph.h @@ -694,6 +694,7 @@ class Loop // Temporary map to reuse existing startIndexOpnd while emitting // 0 = !increment & !alreadyChanged, 1 = !increment & alreadyChanged, 2 = increment & !alreadyChanged, 3 = increment & alreadyChanged IR::RegOpnd* startIndexOpndCache[4]; + IR::Instr* instr; } MemOpInfo; bool doMemOp : 1; diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index 3087b08d586..78651f93bbb 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -16854,6 +16854,9 @@ GlobOpt::EmitMemop(Loop * loop, LoopCount *loopCount, const MemOpEmitData* emitD memopInstr->SetSrc2(sizeOpnd); insertBeforeInstr->InsertBefore(memopInstr); + + loop->memOpInfo->instr = memopInstr; + #if DBG_DUMP if (DO_MEMOP_TRACE()) { From 5db42187c3fd129c9574d61ceb7236d932bb69dc Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Wed, 14 Nov 2018 12:40:41 -0800 Subject: [PATCH 3/5] CVE-2018-8618 Edge - Report a type confusion bug --- lib/Backend/GlobOpt.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index 78651f93bbb..0e078051043 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -1840,6 +1840,10 @@ GlobOpt::IsAllowedForMemOpt(IR::Instr* instr, bool isMemset, IR::RegOpnd *baseOp return false; } } + else + { + return false; + } if (!baseValueType.IsTypedArray()) { From 69a259c8c3993b23a9e33772fc5a5bfd22466bd5 Mon Sep 17 00:00:00 2001 From: Rajat Dua Date: Wed, 14 Nov 2018 15:13:23 -0800 Subject: [PATCH 4/5] CVE-2018-8629 OOB bug in Edge WIP --- lib/Backend/FlowGraph.cpp | 8 +++++++- lib/Backend/FlowGraph.h | 4 +++- lib/Backend/GlobOpt.cpp | 11 ++++++++--- lib/Backend/GlobOpt.h | 2 +- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/Backend/FlowGraph.cpp b/lib/Backend/FlowGraph.cpp index d8841ee8c3c..5b567775daa 100644 --- a/lib/Backend/FlowGraph.cpp +++ b/lib/Backend/FlowGraph.cpp @@ -5266,7 +5266,7 @@ BasicBlock::MergePredBlocksValueMaps(GlobOpt* globOpt) } if(symsRequiringCompensationToMergedValueInfoMap.Count() != 0) { - globOpt->InsertValueCompensation(pred, symsRequiringCompensationToMergedValueInfoMap); + globOpt->InsertValueCompensation(pred, &symsRequiringCompensationToMergedValueInfoMap); } } } NEXT_PREDECESSOR_EDGE_EDITING; @@ -5325,6 +5325,12 @@ BasicBlock::MergePredBlocksValueMaps(GlobOpt* globOpt) loop->liveFieldsOnEntry = JitAnew(globOpt->alloc, BVSparse, globOpt->alloc); loop->liveFieldsOnEntry->Copy(this->globOptData.liveFields); + if (symsRequiringCompensationToMergedValueInfoMap.Count() != 0) + { + loop->symsRequiringCompensationToMergedValueInfoMap = JitAnew(globOpt->alloc, SymToValueInfoMap, globOpt->alloc); + loop->symsRequiringCompensationToMergedValueInfoMap->Copy(&symsRequiringCompensationToMergedValueInfoMap); + } + if(globOpt->DoBoundCheckHoist() && loop->inductionVariables) { globOpt->FinalizeInductionVariables(loop, &blockData); diff --git a/lib/Backend/FlowGraph.h b/lib/Backend/FlowGraph.h index bcfbca7c15e..b08fc1d4c50 100644 --- a/lib/Backend/FlowGraph.h +++ b/lib/Backend/FlowGraph.h @@ -575,6 +575,7 @@ class Loop BVSparse *lossyInt32SymsOnEntry; // see GlobOptData::liveLossyInt32Syms BVSparse *float64SymsOnEntry; BVSparse *liveFieldsOnEntry; + SymToValueInfoMap *symsRequiringCompensationToMergedValueInfoMap; BVSparse *symsUsedBeforeDefined; // stack syms that are live in the landing pad, and used before they are defined in the loop BVSparse *likelyIntSymsUsedBeforeDefined; // stack syms that are live in the landing pad with a likely-int value, and used before they are defined in the loop @@ -742,7 +743,8 @@ class Loop allFieldsKilled(false), isLeaf(true), isProcessed(false), - initialValueFieldMap(alloc) + initialValueFieldMap(alloc), + symsRequiringCompensationToMergedValueInfoMap(nullptr) { this->loopNumber = ++func->loopCount; } diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index 0e078051043..e82439fb50b 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -599,6 +599,11 @@ GlobOpt::OptBlock(BasicBlock *block) this->tempBv->And(liveOnBackEdge); this->ToFloat64(this->tempBv, block->loop->landingPad); + if (block->loop->symsRequiringCompensationToMergedValueInfoMap) + { + InsertValueCompensation(block, block->loop->symsRequiringCompensationToMergedValueInfoMap); + } + // Now that we're done with the liveFields within this loop, trim the set to those syms // that the backward pass told us were live out of the loop. // This assumes we have no further need of the liveFields within the loop. @@ -1151,10 +1156,10 @@ void GlobOpt::FieldPRE(Loop *loop) void GlobOpt::InsertValueCompensation( BasicBlock *const predecessor, - const SymToValueInfoMap &symsRequiringCompensationToMergedValueInfoMap) + const SymToValueInfoMap *symsRequiringCompensationToMergedValueInfoMap) { Assert(predecessor); - Assert(symsRequiringCompensationToMergedValueInfoMap.Count() != 0); + Assert(symsRequiringCompensationToMergedValueInfoMap->Count() != 0); IR::Instr *insertBeforeInstr = predecessor->GetLastInstr(); Func *const func = insertBeforeInstr->m_func; @@ -1193,7 +1198,7 @@ void GlobOpt::InsertValueCompensation( } }; JsUtil::List delayChangeValueInfo(alloc); - for(auto it = symsRequiringCompensationToMergedValueInfoMap.GetIterator(); it.IsValid(); it.MoveNext()) + for(auto it = symsRequiringCompensationToMergedValueInfoMap->GetIterator(); it.IsValid(); it.MoveNext()) { const auto &entry = it.Current(); Sym *const sym = entry.Key(); diff --git a/lib/Backend/GlobOpt.h b/lib/Backend/GlobOpt.h index aa673858b2a..0cdcac4feaf 100644 --- a/lib/Backend/GlobOpt.h +++ b/lib/Backend/GlobOpt.h @@ -737,7 +737,7 @@ class GlobOpt void PreLowerCanonicalize(IR::Instr *instr, Value **pSrc1Val, Value **pSrc2Val); void ProcessKills(IR::Instr *instr); void InsertCloneStrs(BasicBlock *toBlock, GlobOptBlockData *toData, GlobOptBlockData *fromData); - void InsertValueCompensation(BasicBlock *const predecessor, const SymToValueInfoMap &symsRequiringCompensationToMergedValueInfoMap); + void InsertValueCompensation(BasicBlock *const predecessor, const SymToValueInfoMap *symsRequiringCompensationToMergedValueInfoMap); IR::Instr * ToVarUses(IR::Instr *instr, IR::Opnd *opnd, bool isDst, Value *val); void ToVar(BVSparse *bv, BasicBlock *block); IR::Instr * ToVar(IR::Instr *instr, IR::RegOpnd *regOpnd, BasicBlock *block, Value *val, bool needsUpdate); From c04787f16efe8564cd3acee7549854dc156419b2 Mon Sep 17 00:00:00 2001 From: Paul Leathers Date: Thu, 15 Nov 2018 11:01:21 -0800 Subject: [PATCH 5/5] CVE-2018-8617 --- lib/Backend/GlobOpt.cpp | 6 ------ lib/Backend/GlobOptFields.cpp | 28 +++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index e82439fb50b..3f636e9cf7e 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -2852,12 +2852,6 @@ GlobOpt::OptDst( { this->FinishOptPropOp(instr, opnd->AsPropertySymOpnd()); } - else if (instr->m_opcode == Js::OpCode::StElemI_A || - instr->m_opcode == Js::OpCode::StElemI_A_Strict || - instr->m_opcode == Js::OpCode::InitComputedProperty) - { - this->KillObjectHeaderInlinedTypeSyms(this->currentBlock, false); - } if (opnd->IsIndirOpnd() && !this->IsLoopPrePass()) { diff --git a/lib/Backend/GlobOptFields.cpp b/lib/Backend/GlobOptFields.cpp index 85c7fb35ac7..fade2e3e331 100644 --- a/lib/Backend/GlobOptFields.cpp +++ b/lib/Backend/GlobOptFields.cpp @@ -328,10 +328,20 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse *bv, bo Assert(dstOpnd != nullptr); KillLiveFields(this->lengthEquivBv, bv); KillLiveElems(dstOpnd->AsIndirOpnd(), bv, inGlobOpt, instr->m_func); + if (inGlobOpt) + { + KillObjectHeaderInlinedTypeSyms(this->currentBlock, false); + } break; case Js::OpCode::InitComputedProperty: + case Js::OpCode::InitGetElemI: + case Js::OpCode::InitSetElemI: KillLiveElems(dstOpnd->AsIndirOpnd(), bv, inGlobOpt, instr->m_func); + if (inGlobOpt) + { + KillObjectHeaderInlinedTypeSyms(this->currentBlock, false); + } break; case Js::OpCode::DeleteElemI_A: @@ -394,6 +404,10 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse *bv, bo case Js::OpCode::InlineArrayPush: case Js::OpCode::InlineArrayPop: KillLiveFields(this->lengthEquivBv, bv); + if (inGlobOpt) + { + KillObjectHeaderInlinedTypeSyms(this->currentBlock, false); + } break; case Js::OpCode::InlineeStart: @@ -410,10 +424,18 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse *bv, bo fnHelper = instr->GetSrc1()->AsHelperCallOpnd()->m_fnHelper; // Kill length field for built-ins that can update it. - if(nullptr != this->lengthEquivBv && (fnHelper == IR::JnHelperMethod::HelperArray_Shift || fnHelper == IR::JnHelperMethod::HelperArray_Splice - || fnHelper == IR::JnHelperMethod::HelperArray_Unshift)) + if(fnHelper == IR::JnHelperMethod::HelperArray_Shift + || fnHelper == IR::JnHelperMethod::HelperArray_Splice + || fnHelper == IR::JnHelperMethod::HelperArray_Unshift) { - KillLiveFields(this->lengthEquivBv, bv); + if (nullptr != this->lengthEquivBv) + { + KillLiveFields(this->lengthEquivBv, bv); + } + if (inGlobOpt) + { + KillObjectHeaderInlinedTypeSyms(this->currentBlock, false); + } } if ((fnHelper == IR::JnHelperMethod::HelperRegExp_Exec)