[CVE-2019-0924]

chakra-core · May 13, 2019 · 6615113 · 6615113
1 parent a9ab1aa
commit 6615113
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 18 deletions.
diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp
@@ -4677,10 +4677,9 @@ BackwardPass::ProcessNewScObject(IR::Instr* instr)
         return;
     }
 
-    if (instr->HasBailOutInfo())
+    if (instr->HasBailOutInfo() && (instr->GetBailOutKind() & ~IR::BailOutKindBits) == IR::BailOutFailedCtorGuardCheck)
     {
         Assert(instr->IsProfiledInstr());
-        Assert(instr->GetBailOutKind() == IR::BailOutFailedCtorGuardCheck);
         Assert(instr->GetDst()->IsRegOpnd());
 
         BasicBlock * block = this->currentBlock;

diff --git a/lib/Backend/GlobOptBailOut.cpp b/lib/Backend/GlobOptBailOut.cpp
@@ -1337,6 +1337,14 @@ GlobOpt::MayNeedBailOnImplicitCall(IR::Instr const * instr, Value const * src1Va
             );
     }
 
+    case Js::OpCode::NewScObjectNoCtor:
+        if (instr->HasBailOutInfo() && (instr->GetBailOutKind() & ~IR::BailOutKindBits) == IR::BailOutFailedCtorGuardCheck)
+        {
+            // No helper call with this bailout.
+            return false;
+        }
+        break;
+
     default:
         break;
     }

diff --git a/lib/Backend/Inline.cpp b/lib/Backend/Inline.cpp
@@ -4204,6 +4204,8 @@ Inline::SplitConstructorCallCommon(
     {
         createObjInstr->SetByteCodeOffset(newObjInstr);
         createObjInstr->GetSrc1()->SetIsJITOptimizedReg(true);
+        // We're splitting a single byte code, so the interpreter has to resume from the beginning if we bail out.
+        createObjInstr->forcePreOpBailOutIfNeeded = true;
         newObjInstr->InsertBefore(createObjInstr);
 
         createObjDst->SetValueType(ValueType::GetObject(ObjectType::UninitializedObject));

diff --git a/lib/Backend/Lower.cpp b/lib/Backend/Lower.cpp
@@ -4601,18 +4601,40 @@ Lowerer::LowerNewScObject(IR::Instr *newObjInstr, bool callCtor, bool hasArgs, b
         {
             Assert(!newObjDst->CanStoreTemp());
             // createObjDst = NewScObject...(ctorOpnd)
-            newScHelper = !callCtor ?
-                (isBaseClassConstructorNewScObject ?
-                    (hasArgs ? IR::HelperNewScObjectNoCtorFull : IR::HelperNewScObjectNoArgNoCtorFull) :
-                    (hasArgs ? IR::HelperNewScObjectNoCtor : IR::HelperNewScObjectNoArgNoCtor)) :
-                (hasArgs || usedFixedCtorCache ? IR::HelperNewScObjectNoCtor : IR::HelperNewScObjectNoArg);
 
             LoadScriptContext(newObjInstr);
-            m_lowererMD.LoadHelperArgument(newObjInstr, newObjInstr->GetSrc1());
 
-            newScObjCall = IR::Instr::New(Js::OpCode::Call, createObjDst, IR::HelperCallOpnd::New(newScHelper, func), func);
-            newObjInstr->InsertBefore(newScObjCall);
-            m_lowererMD.LowerCall(newScObjCall, 0);
+            if (callCtor)
+            {
+                newScHelper = (hasArgs || usedFixedCtorCache ? IR::HelperNewScObjectNoCtor : IR::HelperNewScObjectNoArg);
+
+                m_lowererMD.LoadHelperArgument(newObjInstr, newObjInstr->GetSrc1());
+
+                newScObjCall = IR::Instr::New(Js::OpCode::Call, createObjDst, IR::HelperCallOpnd::New(newScHelper, func), func);
+                newObjInstr->InsertBefore(newScObjCall);
+                m_lowererMD.LowerCall(newScObjCall, 0);
+            }
+            else
+            {
+                newScHelper = 
+                    (isBaseClassConstructorNewScObject ?
+                        (hasArgs ? IR::HelperNewScObjectNoCtorFull : IR::HelperNewScObjectNoArgNoCtorFull) :
+                        (hasArgs ? IR::HelperNewScObjectNoCtor : IR::HelperNewScObjectNoArgNoCtor));
+
+                // Branch around the helper call to execute the inlined ctor.
+                Assert(callCtorLabel != nullptr);
+                newObjInstr->InsertAfter(callCtorLabel);
+
+                // Change the NewScObject* to a helper call on the spot. This generates implicit call bailout for us if we need one.
+                m_lowererMD.LoadHelperArgument(newObjInstr, newObjInstr->UnlinkSrc1());
+                m_lowererMD.ChangeToHelperCall(newObjInstr, newScHelper);
+
+                // Then we're done.
+                Assert(createObjDst == newObjDst);
+
+                // Return the first instruction above the region we've just lowered.
+                return RemoveLoweredRegionStartMarker(startMarkerInstr);                    
+            }
         }
     }
 
@@ -4857,21 +4879,17 @@ bool Lowerer::TryLowerNewScObjectWithFixedCtorCache(IR::Instr* newObjInstr, IR::
     skipNewScObj = false;
     returnNewScObj = false;
 
-    AssertMsg(!PHASE_OFF(Js::ObjTypeSpecNewObjPhase, this->m_func) || !newObjInstr->HasBailOutInfo(),
-        "Why do we have bailout on NewScObject when ObjTypeSpecNewObj is off?");
-
     if (PHASE_OFF(Js::FixedNewObjPhase, newObjInstr->m_func) && PHASE_OFF(Js::ObjTypeSpecNewObjPhase, this->m_func))
     {
         return false;
     }
 
     JITTimeConstructorCache * ctorCache;
 
-    if (newObjInstr->HasBailOutInfo())
+    if (newObjInstr->HasBailOutInfo() && (newObjInstr->GetBailOutKind() & ~IR::BailOutKindBits) == IR::BailOutFailedCtorGuardCheck)
     {
         Assert(newObjInstr->IsNewScObjectInstr());
         Assert(newObjInstr->IsProfiledInstr());
-        Assert(newObjInstr->GetBailOutKind() == IR::BailOutFailedCtorGuardCheck);
 
         emitBailOut = true;
 

diff --git a/lib/Runtime/ByteCode/OpCodes.h b/lib/Runtime/ByteCode/OpCodes.h
@@ -577,8 +577,8 @@ MACRO_EXTEND_WMS_AND_PROFILED(NewScObjectSpread,   CallIExtended, OpSideEffect|O
 MACRO_WMS_PROFILED2(    NewScObjArray,      CallI,          OpSideEffect|OpUseAllFields|OpCallInstr)        // Create new ScriptObject instance
 MACRO_WMS_PROFILED2(    NewScObjArraySpread, CallIExtended, OpSideEffect|OpUseAllFields|OpCallInstr)        // Create new ScriptObject instance
 MACRO(                  NewScObject_A,      Auxiliary,      OpSideEffect|OpUseAllFields)                    // Create new ScriptObject instance passing only constants
-MACRO_WMS(              NewScObjectNoCtorFull, Reg2,        OpTempObjectCanStoreTemp)                       // Create new object that will be used for the 'this' binding in a base class constructor
-MACRO_BACKEND_ONLY(     NewScObjectNoCtor,  Empty,          OpTempObjectCanStoreTemp)                       // Create new object that will be passed into a constructor
+MACRO_WMS(              NewScObjectNoCtorFull, Reg2,        OpTempObjectCanStoreTemp|OpHasImplicitCall)     // Create new object that will be used for the 'this' binding in a base class constructor
+MACRO_BACKEND_ONLY(     NewScObjectNoCtor,  Empty,          OpTempObjectCanStoreTemp|OpHasImplicitCall)     // Create new object that will be passed into a constructor
 MACRO_BACKEND_ONLY(     GetNewScObject,     Empty,          OpTempObjectTransfer)                           // Determine which object to finally use as the result of NewScObject (object passed into constructor as 'this', or object returned by constructor)
 MACRO_BACKEND_ONLY(     UpdateNewScObjectCache, Empty,      None)                                           // Update the cache used for NewScObject
 MACRO_WMS(              NewScObjectSimple,  Reg1,           OpTempObjectCanStoreTemp)