CVE-2019-1308

Author: pleath

lib/Backend/BackwardPass.cpp

@@ -2195,6 +2195,13 @@ BackwardPass::DeadStoreTypeCheckBailOut(IR::Instr * instr)
         return;
     }
 
+    // By default, do not do this for stores, as it makes the presence of type checks unpredictable in the forward pass.
+    // For instance, we can't predict which stores may cause reallocation of aux slots.
+    if (instr->GetDst() && instr->GetDst()->IsSymOpnd())
+    {
+        return;
+    }
+
     IR::BailOutKind oldBailOutKind = instr->GetBailOutKind();
     if (!IR::IsTypeCheckBailOutKind(oldBailOutKind))
     {

lib/Backend/GlobOptFields.cpp

@@ -905,7 +905,7 @@ GlobOpt::FinishOptPropOp(IR::Instr *instr, IR::PropertySymOpnd *opnd, BasicBlock
 
         SymID opndId = opnd->HasObjectTypeSym() ? opnd->GetObjectTypeSym()->m_id : -1;
 
-        if (!isObjTypeChecked)
+        if (!isObjTypeSpecialized || opnd->IsBeingAdded())
         {
             if (block->globOptData.maybeWrittenTypeSyms == nullptr)
             {
@@ -1122,6 +1122,19 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
     Assert(opnd->IsTypeCheckSeqCandidate());
     Assert(opnd->HasObjectTypeSym());
 
+    if (opnd->HasTypeMismatch())
+    {
+        if (emitsTypeCheckOut != nullptr)
+        {
+            *emitsTypeCheckOut = false;
+        }
+        if (changesTypeValueOut != nullptr)
+        {
+            *changesTypeValueOut = false;
+        }
+        return false;
+    }
+
     bool isStore = opnd == instr->GetDst();
     bool isTypeDead = opnd->IsTypeDead();
     bool consumeType = makeChanges && !IsLoopPrePass();
@@ -1229,7 +1242,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
                 // a new type value here.
                 isSpecialized = false;
 
-                if (consumeType)
+                if (makeChanges)
                 {
                     opnd->SetTypeMismatch(true);
                 }
@@ -1273,7 +1286,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
                 // a new type value here.
                 isSpecialized = false;
 
-                if (consumeType)
+                if (makeChanges)
                 {
                     opnd->SetTypeMismatch(true);
                 }
@@ -1324,7 +1337,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
             {
                 // Indicates failure/mismatch
                 isSpecialized = false;
-                if (consumeType)
+                if (makeChanges)
                 {
                     opnd->SetTypeMismatch(true);
                 }
@@ -1423,7 +1436,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
             // a new type value here.
             isSpecialized = false;
 
-            if (consumeType)
+            if (makeChanges)
             {
                 opnd->SetTypeMismatch(true);
             }

lib/Backend/Lower.cpp

@@ -7420,9 +7420,6 @@ Lowerer::GenerateStFldWithCachedType(IR::Instr *instrStFld, bool* continueAsHelp
 
     if (hasTypeCheckBailout)
     {
-        AssertMsg(PHASE_ON1(Js::ObjTypeSpecIsolatedFldOpsWithBailOutPhase) || !propertySymOpnd->IsTypeDead() || propertySymOpnd->TypeCheckRequired(),
-            "Why does a field store have a type check bailout, if its type is dead?");
-
         if (instrStFld->GetBailOutInfo()->bailOutInstr != instrStFld)
         {
             // Set the cache index in the bailout info so that the generated code will write it into the
@@ -7482,7 +7479,7 @@ Lowerer::GenerateCachedTypeCheck(IR::Instr *instrChk, IR::PropertySymOpnd *prope
     // cache and no type check bailout. In the latter case, we can wind up doing expensive failed equivalence checks
     // repeatedly and never rejit.
     bool doEquivTypeCheck =
-        (instrChk->HasEquivalentTypeCheckBailOut() && propertySymOpnd->TypeCheckRequired()) ||
+        (instrChk->HasEquivalentTypeCheckBailOut() && (propertySymOpnd->TypeCheckRequired() || propertySymOpnd == instrChk->GetDst())) ||
         (propertySymOpnd->HasEquivalentTypeSet() &&
          !(propertySymOpnd->HasFinalType() && propertySymOpnd->HasInitialType()) &&
          !propertySymOpnd->MustDoMonoCheck() &&

test/fieldopts/OS23440664.js

@@ -0,0 +1,17 @@
+//Reduced Switches: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:1 -werexceptionsupport -oopjit- -bvt -off:bailonnoprofile -force:fixdataprops -forcejitloopbody 
+var shouldBailout = false;
+var IntArr0 = [];
+function test0() {
+  var loopInvariant = shouldBailout;
+  function makeArrayLength() {
+    return Math.floor();
+  }
+  makeArrayLength();
+  makeArrayLength();
+  prop0 = 1;
+  Object;
+  for (; shouldBailout ? (Object()) : (IntArr0[Object & 1] = '') ? Object : 0;) {
+  }
+}
+test0();
+WScript.Echo('pass');

test/fieldopts/rlexe.xml

@@ -852,4 +852,10 @@
       <files>argobjlengthhoist.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>OS23440664.js</files>
+      <compile-flags>-off:bailonnoprofile -force:fixdataprops -forcejitloopbody</compile-flags>
+    </default>
+  </test>
 </regress-exe>