[MERGE #5764 @thomasmo] October 2018 Security Update

Thomas Moore (CHAKRA) · Thomas Moore (CHAKRA) · commit f40b55cf266e · 2018-10-09T12:18:17.000-07:00
Merge pull request #5764 from thomasmo:1810 October 2018 Security Update that addresses the following issues in ChakraCore: CVE-2018-8473 CVE-2018-8500 CVE-2018-8503 CVE-2018-8505 CVE-2018-8510 CVE-2018-8511 CVE-2018-8513
diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp
@@ -1013,28 +1013,17 @@ BOOL GlobOpt::PRE::PreloadPRECandidate(Loop *loop, GlobHashBucket* candidate)
     // Create instr to put in landing pad for compensation
     Assert(IsPREInstrCandidateLoad(ldInstrInLoop->m_opcode));
 
-    IR::Instr * ldInstr = InsertPropertySymPreloadWithoutDstInLandingPad(ldInstrInLoop, loop, propertySym);
+    IR::Instr * ldInstr = InsertPropertySymPreloadInLandingPad(ldInstrInLoop, loop, propertySym);
     if (!ldInstr)
     {
         return false;
     }
 
     Assert(ldInstr->GetDst() == nullptr);
-    if (ldInstrInLoop->GetDst())
-    {
-        Assert(ldInstrInLoop->GetDst()->IsRegOpnd());
-        if (ldInstrInLoop->GetDst()->AsRegOpnd()->m_sym != symStore)
-        {
-            ldInstr->SetDst(IR::RegOpnd::New(symStore->AsStackSym(), TyVar, this->globOpt->func));
-            loop->fieldPRESymStores->Set(symStore->m_id);
-        }
-        else
-        {
-            ldInstr->SetDst(ldInstrInLoop->GetDst()->Copy(ldInstrInLoop->m_func));
-        }
-        landingPad->globOptData.liveVarSyms->Set(ldInstr->GetDst()->AsRegOpnd()->m_sym->m_id);
-    }
-
+    ldInstr->SetDst(IR::RegOpnd::New(symStore->AsStackSym(), TyVar, this->globOpt->func));
+    loop->fieldPRESymStores->Set(symStore->m_id);
+    landingPad->globOptData.liveVarSyms->Set(symStore->m_id);
+    
     Value * objPtrValue = landingPad->globOptData.FindValue(objPtrSym);
 
     objPtrCopyPropSym = objPtrCopyPropSym ? objPtrCopyPropSym : objPtrValue ? landingPad->globOptData.GetCopyPropSym(objPtrSym, objPtrValue) : nullptr;
@@ -3310,7 +3299,7 @@ GlobOpt::OptSrc(IR::Opnd *opnd, IR::Instr * *pInstr, Value **indirIndexValRef, I
                 // Can this be done in one call?
                 if (!this->prePassInstrMap->ContainsKey(sym->m_id))
                 {
-                    this->prePassInstrMap->AddNew(sym->m_id, instr);
+                    this->prePassInstrMap->AddNew(sym->m_id, instr->CopyWithoutDst());
                 }
             }
         }
@@ -17235,7 +17224,7 @@ GlobOpt::PRE::InsertSymDefinitionInLandingPad(StackSym * sym, Loop * loop, Sym *
                 // #1 is done next. #2 and #3 are done as part of preloading T1.y
 
                 // Insert T1 = o.x
-                if (!InsertPropertySymPreloadInLandingPad(symDefInstr, loop, propSym))
+                if (!InsertPropertySymPreloadInLandingPad(symDefInstr->Copy(), loop, propSym))
                 {
                     return false;
                 }
@@ -17248,7 +17237,7 @@ GlobOpt::PRE::InsertSymDefinitionInLandingPad(StackSym * sym, Loop * loop, Sym *
                 if (loop->landingPad->globOptData.IsLive(*objPtrCopyPropSym))
                 {
                     // insert T1 = o.x
-                    if (!InsertPropertySymPreloadInLandingPad(symDefInstr, loop, propSym))
+                    if (!InsertPropertySymPreloadInLandingPad(symDefInstr->Copy(), loop, propSym))
                     {
                         return false;
                     }
@@ -17336,25 +17325,6 @@ GlobOpt::PRE::InsertInstrInLandingPad(IR::Instr * instr, Loop * loop)
 
 IR::Instr *
 GlobOpt::PRE::InsertPropertySymPreloadInLandingPad(IR::Instr * ldInstr, Loop * loop, PropertySym * propertySym)
-{
-    IR::Instr * instr = InsertPropertySymPreloadWithoutDstInLandingPad(ldInstr, loop, propertySym);
-    if (!instr)
-    {
-        return nullptr;
-    }
-
-    if (ldInstr->GetDst())
-    {
-        instr->SetDst(ldInstr->GetDst()->Copy(ldInstr->m_func));
-        instr->GetDst()->SetIsJITOptimizedReg(true);
-        loop->landingPad->globOptData.liveVarSyms->Set(instr->GetDst()->GetStackSym()->m_id);
-    }
-
-    return instr;
-}
-
-IR::Instr *
-GlobOpt::PRE::InsertPropertySymPreloadWithoutDstInLandingPad(IR::Instr * ldInstr, Loop * loop, PropertySym * propertySym)
 {
     IR::SymOpnd *ldSrc = ldInstr->GetSrc1()->AsSymOpnd();
 
@@ -17369,8 +17339,6 @@ GlobOpt::PRE::InsertPropertySymPreloadWithoutDstInLandingPad(IR::Instr * ldInstr
         }
     }
 
-    ldInstr = ldInstr->CopyWithoutDst();
-
     // Consider: Shouldn't be necessary once we have copy-prop in prepass...
     ldInstr->GetSrc1()->AsSymOpnd()->m_sym = propertySym;
     ldSrc = ldInstr->GetSrc1()->AsSymOpnd();
@@ -17384,6 +17352,11 @@ GlobOpt::PRE::InsertPropertySymPreloadWithoutDstInLandingPad(IR::Instr * ldInstr
         ldInstr->ReplaceSrc1(newPropSymOpnd);
     }
 
+    if (ldInstr->GetDst())
+    {
+        loop->landingPad->globOptData.liveVarSyms->Set(ldInstr->GetDst()->GetStackSym()->m_id);
+    }
+
     InsertInstrInLandingPad(ldInstr, loop);
 
     return ldInstr;
diff --git a/lib/Backend/GlobOpt.h b/lib/Backend/GlobOpt.h
@@ -1029,7 +1029,6 @@ class GlobOpt::PRE
     void                    FindPossiblePRECandidates(Loop *loop, JitArenaAllocator *alloc);
     void                    PreloadPRECandidates(Loop *loop);
     BOOL                    PreloadPRECandidate(Loop *loop, GlobHashBucket* candidate);
-    IR::Instr *             InsertPropertySymPreloadWithoutDstInLandingPad(IR::Instr * origLdInstr, Loop * loop, PropertySym * propertySym);
     IR::Instr *             InsertPropertySymPreloadInLandingPad(IR::Instr * origLdInstr, Loop * loop, PropertySym * propertySym);
     void                    InsertInstrInLandingPad(IR::Instr * instr, Loop * loop);
     bool                    InsertSymDefinitionInLandingPad(StackSym * sym, Loop * loop, Sym ** objPtrCopyPropSym);
diff --git a/lib/Backend/GlobOptArrays.cpp b/lib/Backend/GlobOptArrays.cpp
@@ -320,7 +320,8 @@ void GlobOpt::ArraySrcOpt::CheckVirtualArrayBounds()
                 {
                     Assert(instr->m_opcode == Js::OpCode::InlineArrayPush ||
                         instr->m_opcode == Js::OpCode::InlineArrayPop ||
-                        instr->m_opcode == Js::OpCode::LdLen_A);
+                        instr->m_opcode == Js::OpCode::LdLen_A ||
+                        instr->m_opcode == Js::OpCode::IsIn);
                 }
 
                 eliminatedLowerBoundCheck = true;
@@ -1988,6 +1989,8 @@ void GlobOpt::ArraySrcOpt::Optimize()
         {
             TRACE_TESTTRACE_PHASE_INSTR(Js::Phase::BoundCheckEliminationPhase, instr, _u("Eliminating IsIn\n"));
 
+            globOpt->CaptureByteCodeSymUses(instr);
+
             instr->m_opcode = Js::OpCode::Ld_A;
 
             IR::AddrOpnd * addrOpnd = IR::AddrOpnd::New(func->GetScriptContextInfo()->GetTrueAddr(), IR::AddrOpndKindDynamicVar, func, true);
diff --git a/lib/Backend/GlobOptBailOut.cpp b/lib/Backend/GlobOptBailOut.cpp
@@ -583,6 +583,8 @@ GlobOpt::TrackCalls(IR::Instr * instr)
             instr->m_func->m_hasInlineArgsOpt = true;
             InlineeFrameInfo* frameInfo = InlineeFrameInfo::New(func->m_alloc);
             instr->m_func->frameInfo = frameInfo;
+            frameInfo->functionSymStartValue = instr->GetSrc1()->GetSym() ?
+                CurrentBlockData()->FindValue(instr->GetSrc1()->GetSym()) : nullptr;
             frameInfo->floatSyms = CurrentBlockData()->liveFloat64Syms->CopyNew(this->alloc);
             frameInfo->intSyms = CurrentBlockData()->liveInt32Syms->MinusNew(CurrentBlockData()->liveLossyInt32Syms, this->alloc);
             frameInfo->varSyms = CurrentBlockData()->liveVarSyms->CopyNew(this->alloc);
@@ -762,6 +764,15 @@ void GlobOpt::RecordInlineeFrameInfo(IR::Instr* inlineeEnd)
             }
             else
             {
+                // If the value of the functionObject symbol has changed between the inlineeStart and the inlineeEnd,
+                // we don't record the inlinee frame info (see OS#18318884).
+                Assert(frameInfo->functionSymStartValue != nullptr);
+                if (!frameInfo->functionSymStartValue->IsEqualTo(CurrentBlockData()->FindValue(functionObject->m_sym)))
+                {
+                    argInstr->m_func->DisableCanDoInlineArgOpt();
+                    return true;
+                }
+
                 frameInfo->function = InlineFrameInfoValue(functionObject->m_sym);
             }
         }
diff --git a/lib/Backend/InlineeFrameInfo.h b/lib/Backend/InlineeFrameInfo.h
@@ -5,6 +5,8 @@
 
 #pragma once
 
+class Value;
+
 struct BailoutConstantValue {
 public:
     void InitIntConstValue(int32 value) { this->type = TyInt32; this->u.intConst.value = (IntConstType)value; };
@@ -150,6 +152,7 @@ struct InlineeFrameInfo
     BVSparse<JitArenaAllocator>* floatSyms;
     BVSparse<JitArenaAllocator>* intSyms;
     BVSparse<JitArenaAllocator>* varSyms;
+    Value* functionSymStartValue;
 
     bool isRecorded;
 
diff --git a/lib/Runtime/Base/FunctionBody.cpp b/lib/Runtime/Base/FunctionBody.cpp
@@ -2105,7 +2105,7 @@ namespace Js
     {
         FunctionTypeWeakRefList* typeList = EnsureFunctionObjectTypeList();
 
-        Assert(functionType != deferredPrototypeType);
+        Assert(functionType != deferredPrototypeType && functionType != undeferredFunctionType);
         Recycler * recycler = this->GetScriptContext()->GetRecycler();
         FunctionTypeWeakRef* weakRef = recycler->CreateWeakReferenceHandle(functionType);
         typeList->SetAtFirstFreeSpot(weakRef);
diff --git a/lib/Runtime/Base/FunctionBody.h b/lib/Runtime/Base/FunctionBody.h
@@ -1088,8 +1088,10 @@ namespace Js
             {
                 func(this->deferredPrototypeType);
             }
-            // NOTE: We deliberately do not map the undeferredFunctionType here, since it's in the list
-            // of registered function object types we processed above.
+            if (this->undeferredFunctionType)
+            {
+                func(this->undeferredFunctionType);
+            }
         }
 
         static uint GetOffsetOfDeferredPrototypeType() { return static_cast<uint>(offsetof(Js::FunctionProxy, deferredPrototypeType)); }
diff --git a/lib/Runtime/Language/JavascriptOperators.cpp b/lib/Runtime/Language/JavascriptOperators.cpp
@@ -4950,13 +4950,18 @@ using namespace Js;
                 }
                 else if (instanceType == TypeIds_NativeIntArray)
                 {
-                    // Only accept tagged int. Also covers case for MissingItem
+                    // Only accept tagged int.
                     if (!TaggedInt::Is(value))
                     {
                         return false;
                     }
                     int32 intValue = 0;
                     if (!MemsetConversion<int32, JavascriptConversion::ToInt32>(value, scriptContext, &intValue))
+                    {
+                        return false;
+                    }
+                     // Special case for missing item
+                    if (SparseArraySegment<int32>::IsMissingItem(&intValue))
                     {
                         return false;
                     }
diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp
@@ -1499,13 +1499,11 @@ using namespace Js;
 
             bool isTaggedInt = TaggedInt::Is(item);
             bool isTaggedIntMissingValue = false;
-#ifdef TARGET_64
             if (isTaggedInt)
             {
                 int32 iValue = TaggedInt::ToInt32(item);
                 isTaggedIntMissingValue = Js::SparseArraySegment<int32>::IsMissingItem(&iValue);
             }
-#endif
             if (isTaggedInt && !isTaggedIntMissingValue)
             {
                 // This is taggedInt case and we verified that item is not missing value in AMD64.
@@ -3426,16 +3424,12 @@ using namespace Js;
             {
                 if (TaggedInt::Is(aItem))
                 {
-                    pDestArray->DirectSetItemAt(idxDest, TaggedInt::ToInt32(aItem));
+                    int32 int32Value = TaggedInt::ToInt32(aItem);
+                    Assert(!SparseArraySegment<int32>::IsMissingItem(&int32Value));
+                    pDestArray->DirectSetItemAt(idxDest, int32Value);
                 }
                 else
                 {
-#if DBG
-                    int32 int32Value;
-                    Assert(
-                        JavascriptNumber::TryGetInt32Value(JavascriptNumber::GetValue(aItem), &int32Value) &&
-                        !SparseArraySegment<int32>::IsMissingItem(&int32Value));
-#endif
                     pDestArray->DirectSetItemAt(idxDest, static_cast<int32>(JavascriptNumber::GetValue(aItem)));
                 }
                 ++idxDest;
diff --git a/lib/Runtime/Types/PathTypeHandler.cpp b/lib/Runtime/Types/PathTypeHandler.cpp
@@ -1476,6 +1476,9 @@ namespace Js
                 if (!(attributes[descriptor->GetDataPropertyIndex<false>()] & ObjectSlotAttr_Accessor))
                 {
                     // Setter without a getter; this is a stale entry, so ignore it
+                    // Just consume the slot so no descriptor refers to it.
+                    Assert(i == newTypeHandler->nextPropertyIndex);
+                    ::Math::PostInc(newTypeHandler->nextPropertyIndex);
                     continue;
                 }
                 Assert(oldTypeHandler->GetSetterSlotIndex(descriptor->GetDataPropertyIndex<false>()) == newTypeHandler->nextPropertyIndex);

Original file line number	Diff line number	Diff line change
`@@ -320,7 +320,8 @@ void GlobOpt::ArraySrcOpt::CheckVirtualArrayBounds()`
`320`	`320`	`{`
`321`	`321`	`Assert(instr->m_opcode == Js::OpCode::InlineArrayPush \|\|`
`322`	`322`	`instr->m_opcode == Js::OpCode::InlineArrayPop \|\|`
`323`		`- instr->m_opcode == Js::OpCode::LdLen_A);`
	`323`	`+ instr->m_opcode == Js::OpCode::LdLen_A \|\|`
	`324`	`+ instr->m_opcode == Js::OpCode::IsIn);`
`324`	`325`	`}`
`325`	`326`
`326`	`327`	`eliminatedLowerBoundCheck = true;`
`@@ -1988,6 +1989,8 @@ void GlobOpt::ArraySrcOpt::Optimize()`
`1988`	`1989`	`{`
`1989`	`1990`	`TRACE_TESTTRACE_PHASE_INSTR(Js::Phase::BoundCheckEliminationPhase, instr, _u("Eliminating IsIn\n"));`
`1990`	`1991`
	`1992`	`+ globOpt->CaptureByteCodeSymUses(instr);`
	`1993`	`+`
`1991`	`1994`	`instr->m_opcode = Js::OpCode::Ld_A;`
`1992`	`1995`
`1993`	`1996`	`IR::AddrOpnd * addrOpnd = IR::AddrOpnd::New(func->GetScriptContextInfo()->GetTrueAddr(), IR::AddrOpndKindDynamicVar, func, true);`
Original file line number	Diff line number	Diff line change
`@@ -583,6 +583,8 @@ GlobOpt::TrackCalls(IR::Instr * instr)`
`583`	`583`	`instr->m_func->m_hasInlineArgsOpt = true;`
`584`	`584`	`InlineeFrameInfo* frameInfo = InlineeFrameInfo::New(func->m_alloc);`
`585`	`585`	`instr->m_func->frameInfo = frameInfo;`
	`586`	`+ frameInfo->functionSymStartValue = instr->GetSrc1()->GetSym() ?`
	`587`	`+ CurrentBlockData()->FindValue(instr->GetSrc1()->GetSym()) : nullptr;`
`586`	`588`	`frameInfo->floatSyms = CurrentBlockData()->liveFloat64Syms->CopyNew(this->alloc);`
`587`	`589`	`frameInfo->intSyms = CurrentBlockData()->liveInt32Syms->MinusNew(CurrentBlockData()->liveLossyInt32Syms, this->alloc);`
`588`	`590`	`frameInfo->varSyms = CurrentBlockData()->liveVarSyms->CopyNew(this->alloc);`
`@@ -762,6 +764,15 @@ void GlobOpt::RecordInlineeFrameInfo(IR::Instr* inlineeEnd)`
`762`	`764`	`}`
`763`	`765`	`else`
`764`	`766`	`{`
	`767`	`+ // If the value of the functionObject symbol has changed between the inlineeStart and the inlineeEnd,`
	`768`	`+ // we don't record the inlinee frame info (see OS#18318884).`
	`769`	`+ Assert(frameInfo->functionSymStartValue != nullptr);`
	`770`	`+ if (!frameInfo->functionSymStartValue->IsEqualTo(CurrentBlockData()->FindValue(functionObject->m_sym)))`
	`771`	`+ {`
	`772`	`+ argInstr->m_func->DisableCanDoInlineArgOpt();`
	`773`	`+ return true;`
	`774`	`+ }`
	`775`	`+`
`765`	`776`	`frameInfo->function = InlineFrameInfoValue(functionObject->m_sym);`
`766`	`777`	`}`
`767`	`778`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2105,7 +2105,7 @@ namespace Js`
`2105`	`2105`	`{`
`2106`	`2106`	`FunctionTypeWeakRefList* typeList = EnsureFunctionObjectTypeList();`
`2107`	`2107`
`2108`		`- Assert(functionType != deferredPrototypeType);`
	`2108`	`+ Assert(functionType != deferredPrototypeType && functionType != undeferredFunctionType);`
`2109`	`2109`	`Recycler * recycler = this->GetScriptContext()->GetRecycler();`
`2110`	`2110`	`FunctionTypeWeakRef* weakRef = recycler->CreateWeakReferenceHandle(functionType);`
`2111`	`2111`	`typeList->SetAtFirstFreeSpot(weakRef);`
Original file line number	Diff line number	Diff line change
`@@ -1088,8 +1088,10 @@ namespace Js`
`1088`	`1088`	`{`
`1089`	`1089`	`func(this->deferredPrototypeType);`
`1090`	`1090`	`}`
`1091`		`- // NOTE: We deliberately do not map the undeferredFunctionType here, since it's in the list`
`1092`		`- // of registered function object types we processed above.`
	`1091`	`+ if (this->undeferredFunctionType)`
	`1092`	`+ {`
	`1093`	`+ func(this->undeferredFunctionType);`
	`1094`	`+ }`
`1093`	`1095`	`}`
`1094`	`1096`
`1095`	`1097`	`static uint GetOffsetOfDeferredPrototypeType() { return static_cast<uint>(offsetof(Js::FunctionProxy, deferredPrototypeType)); }`
Original file line number	Diff line number	Diff line change
`@@ -4950,13 +4950,18 @@ using namespace Js;`
`4950`	`4950`	`}`
`4951`	`4951`	`else if (instanceType == TypeIds_NativeIntArray)`
`4952`	`4952`	`{`
`4953`		`- // Only accept tagged int. Also covers case for MissingItem`
	`4953`	`+ // Only accept tagged int.`
`4954`	`4954`	`if (!TaggedInt::Is(value))`
`4955`	`4955`	`{`
`4956`	`4956`	`return false;`
`4957`	`4957`	`}`
`4958`	`4958`	`int32 intValue = 0;`
`4959`	`4959`	`if (!MemsetConversion<int32, JavascriptConversion::ToInt32>(value, scriptContext, &intValue))`
	`4960`	`+ {`
	`4961`	`+ return false;`
	`4962`	`+ }`
	`4963`	`+ // Special case for missing item`
	`4964`	`+ if (SparseArraySegment<int32>::IsMissingItem(&intValue))`
`4960`	`4965`	`{`
`4961`	`4966`	`return false;`
`4962`	`4967`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1499,13 +1499,11 @@ using namespace Js;`
`1499`	`1499`
`1500`	`1500`	`bool isTaggedInt = TaggedInt::Is(item);`
`1501`	`1501`	`bool isTaggedIntMissingValue = false;`
`1502`		`-#ifdef TARGET_64`
`1503`	`1502`	`if (isTaggedInt)`
`1504`	`1503`	`{`
`1505`	`1504`	`int32 iValue = TaggedInt::ToInt32(item);`
`1506`	`1505`	`isTaggedIntMissingValue = Js::SparseArraySegment<int32>::IsMissingItem(&iValue);`
`1507`	`1506`	`}`
`1508`		`-#endif`
`1509`	`1507`	`if (isTaggedInt && !isTaggedIntMissingValue)`
`1510`	`1508`	`{`
`1511`	`1509`	`// This is taggedInt case and we verified that item is not missing value in AMD64.`
`@@ -3426,16 +3424,12 @@ using namespace Js;`
`3426`	`3424`	`{`
`3427`	`3425`	`if (TaggedInt::Is(aItem))`
`3428`	`3426`	`{`
`3429`		`- pDestArray->DirectSetItemAt(idxDest, TaggedInt::ToInt32(aItem));`
	`3427`	`+ int32 int32Value = TaggedInt::ToInt32(aItem);`
	`3428`	`+ Assert(!SparseArraySegment<int32>::IsMissingItem(&int32Value));`
	`3429`	`+ pDestArray->DirectSetItemAt(idxDest, int32Value);`
`3430`	`3430`	`}`
`3431`	`3431`	`else`
`3432`	`3432`	`{`
`3433`		`-#if DBG`
`3434`		`- int32 int32Value;`
`3435`		`- Assert(`
`3436`		`- JavascriptNumber::TryGetInt32Value(JavascriptNumber::GetValue(aItem), &int32Value) &&`
`3437`		`- !SparseArraySegment<int32>::IsMissingItem(&int32Value));`
`3438`		`-#endif`
`3439`	`3433`	`pDestArray->DirectSetItemAt(idxDest, static_cast<int32>(JavascriptNumber::GetValue(aItem)));`
`3440`	`3434`	`}`
`3441`	`3435`	`++idxDest;`
Original file line number	Diff line number	Diff line change
`@@ -1476,6 +1476,9 @@ namespace Js`
`1476`	`1476`	`if (!(attributes[descriptor->GetDataPropertyIndex<false>()] & ObjectSlotAttr_Accessor))`
`1477`	`1477`	`{`
`1478`	`1478`	`// Setter without a getter; this is a stale entry, so ignore it`
	`1479`	`+ // Just consume the slot so no descriptor refers to it.`
	`1480`	`+ Assert(i == newTypeHandler->nextPropertyIndex);`
	`1481`	`+ ::Math::PostInc(newTypeHandler->nextPropertyIndex);`
`1479`	`1482`	`continue;`
`1480`	`1483`	`}`
`1481`	`1484`	`Assert(oldTypeHandler->GetSetterSlotIndex(descriptor->GetDataPropertyIndex<false>()) == newTypeHandler->nextPropertyIndex);`