[MERGE #3729 @suwc] 17-09 ChakraCore servicing release

Merge pull request #3729 from suwc:build/suwc/1709B_1.7

[CVE-2017-8741]: Limit JSON Stringify Loop to Initialized Portion
[CVE-2017-8748] Fix UAF caused by GC during bailout
[CVE-2017-11767] Do not instantiate param scope if only the function expression symbol is captured
[CVE-2017-8756] JIT peephole optimization error
[CVE-2017-8753] Array Reverse OOM RCE
[CVE-2017-8729] incorrect object pattern.
[CVE-2017-8739] buffer overread IsMissingItem.
[CVE-2017-8751]Type confusion casting undefined with TypeOfPrototypeObjectDictionary type
[CVE-2017-8757]RCE on Windows Insider Preview
[CVE-2017-11764]Parser::ParseCatch doesn't handle "eval"
[CVE-2017-8660] Uninitialized local variables
[CVE-2017-8755] Fail fast if we can't reparse asm.js module after linking failure
[CVE-2017-8649] Bytecode tempering mitigation code accidently turned off - Internal
[CVE-2017-8740] Fix bad byte code gen for 'with'.
[CVE-2017-8752]fix missing bound check in asm.js in case of constant negative index

lib/Backend/BailOut.cpp

@@ -863,12 +863,12 @@ BailOutRecord::RestoreValue(IR::BailOutKind bailOutKind, Js::JavascriptCallStack
         {
             // Register save space (offset is the register number and index into the register save space)
             // Index is one based, so subtract one
-            Js::Var * registerSaveSpace = registerSaves ? registerSaves : (Js::Var *)scriptContext->GetThreadContext()->GetBailOutRegisterSaveSpace();
+            AssertOrFailFast(registerSaves);
 
             if (isFloat64)
             {
                 Assert(RegTypes[LinearScanMD::GetRegisterFromSaveIndex(offset)] == TyFloat64);
-                dblValue = *((double*)&(registerSaveSpace[offset - 1]));
+                dblValue = *((double*)&(registerSaves[offset - 1]));
 #ifdef _M_ARM
                 BAILOUT_VERBOSE_TRACE(newInstance->function->GetFunctionBody(), bailOutKind, _u("Register %-4S  %4d"), RegNames[(offset - RegD0) / 2 + RegD0], offset);
 #else
@@ -883,17 +883,17 @@ BailOutRecord::RestoreValue(IR::BailOutKind bailOutKind, Js::JavascriptCallStack
                     isSimd128B8 || isSimd128B16
                    )
                 {
-                    simdValue = *((SIMDValue *)&(registerSaveSpace[offset - 1]));
+                    simdValue = *((SIMDValue *)&(registerSaves[offset - 1]));
                 }
                 else if (isInt32)
                 {
                     Assert(RegTypes[LinearScanMD::GetRegisterFromSaveIndex(offset)] != TyFloat64);
-                    int32Value = ::Math::PointerCastToIntegralTruncate<int32>(registerSaveSpace[offset - 1]);
+                    int32Value = ::Math::PointerCastToIntegralTruncate<int32>(registerSaves[offset - 1]);
                 }
                 else
                 {
                     Assert(RegTypes[LinearScanMD::GetRegisterFromSaveIndex(offset)] != TyFloat64);
-                    value = registerSaveSpace[offset - 1];
+                    value = registerSaves[offset - 1];
                 }
 
                 BAILOUT_VERBOSE_TRACE(newInstance->function->GetFunctionBody(), bailOutKind, _u("Register %-4S  %4d"), RegNames[LinearScanMD::GetRegisterFromSaveIndex(offset)], offset);
@@ -1169,11 +1169,19 @@ uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::Imp
     // Do not remove the following code.
     // Need to capture the int registers on stack as threadContext->bailOutRegisterSaveSpace is allocated from ThreadAlloc and is not scanned by recycler.
     // We don't want to save float (xmm) registers as they can be huge and they cannot contain a var.
-    Js::Var registerSaves[INT_REG_COUNT];
+    // However, we're somewhat stuck. We need to keep the references around until we restore values, but
+    // we don't have a use for them. The easiest solution is to simply pass this into the corresponding
+    // parameter for BailOutcommonNoCodeGen, but that requires us to save all of the vars, not just the
+    // int ones. This is ultimately significantly more predictable than attempting to manage the lifetimes
+    // in some other way though. We can't just do what we were doing previously, which is saving values
+    // here and not passing them into BailOutCommonNoCodeGen, because then the compiler will likely get
+    // rid of the memcpy and then the dead registerSaves array, since it can figure out that there's no
+    // side effect (due to the GC not being something that the optimizer can, or should, reason about).
+    Js::Var registerSaves[BailOutRegisterSaveSlotCount];
     js_memcpy_s(registerSaves, sizeof(registerSaves), (Js::Var *)layout->functionObject->GetScriptContext()->GetThreadContext()->GetBailOutRegisterSaveSpace(),
         sizeof(registerSaves));
 
-    Js::Var result = BailOutCommonNoCodeGen(layout, bailOutRecord, bailOutOffset, returnAddress, bailOutKind, branchValue, nullptr, bailOutReturnValue, argoutRestoreAddress);
+    Js::Var result = BailOutCommonNoCodeGen(layout, bailOutRecord, bailOutOffset, returnAddress, bailOutKind, branchValue, registerSaves, bailOutReturnValue, argoutRestoreAddress);
     ScheduleFunctionCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), nullptr, bailOutRecord, bailOutKind, bailOutOffset, savedImplicitCallFlags, returnAddress);
     return result;
 }
@@ -1203,7 +1211,14 @@ uint32
 BailOutRecord::BailOutFromLoopBodyCommon(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord, uint32 bailOutOffset,
     IR::BailOutKind bailOutKind, Js::Var branchValue)
 {
-    uint32 result = BailOutFromLoopBodyHelper(layout, bailOutRecord, bailOutOffset, bailOutKind, branchValue);
+    // This isn't strictly necessary if there's no allocations on this path, but because such an
+    // issue would be hard to notice and introduce some significant issues, we can do this copy.
+    // The problem from not doing this and then doing an allocation before RestoreValues is that
+    // the GC doesn't check the BailOutRegisterSaveSpace.
+    Js::Var registerSaves[BailOutRegisterSaveSlotCount];
+    js_memcpy_s(registerSaves, sizeof(registerSaves), (Js::Var *)layout->functionObject->GetScriptContext()->GetThreadContext()->GetBailOutRegisterSaveSpace(),
+        sizeof(registerSaves));
+    uint32 result = BailOutFromLoopBodyHelper(layout, bailOutRecord, bailOutOffset, bailOutKind, branchValue, registerSaves);
     ScheduleLoopBodyCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), nullptr, bailOutRecord, bailOutKind);
     return result;
 }

lib/Backend/BailOut.h

@@ -234,7 +234,7 @@ class BailOutRecord
         Js::RegSlot returnValueRegSlot;
     };
     static Js::Var BailOutCommonNoCodeGen(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord,
-        uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::Var branchValue = nullptr, Js::Var * registerSaves = nullptr,
+        uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::Var branchValue, Js::Var * registerSaves,
         BailOutReturnValue * returnValue = nullptr, void * argoutRestoreAddress = nullptr);
     static Js::Var BailOutCommon(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord,
         uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::ImplicitCallFlags savedImplicitCallFlags, Js::Var branchValue = nullptr, BailOutReturnValue * returnValue = nullptr, void * argoutRestoreAddress = nullptr);
@@ -251,7 +251,7 @@ class BailOutRecord
     static void BailOutInlinedHelper(Js::JavascriptCallStackLayout * layout, BailOutRecord const *& bailOutRecord, uint32 bailOutOffset, void * returnAddress,
         IR::BailOutKind bailOutKind, Js::Var * registerSaves, BailOutReturnValue * returnValue, Js::ScriptFunction ** innerMostInlinee, bool isInLoopBody, Js::Var branchValue = nullptr);
     static uint32 BailOutFromLoopBodyHelper(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord,
-        uint32 bailOutOffset, IR::BailOutKind bailOutKind, Js::Var branchValue = nullptr, Js::Var * registerSaves = nullptr, BailOutReturnValue * returnValue = nullptr);
+        uint32 bailOutOffset, IR::BailOutKind bailOutKind, Js::Var branchValue, Js::Var * registerSaves, BailOutReturnValue * returnValue = nullptr);
 
     static void UpdatePolymorphicFieldAccess(Js::JavascriptFunction *  function, BailOutRecord const * bailOutRecord);
 

lib/Backend/IRBuilder.cpp

@@ -415,6 +415,11 @@ IRBuilder::Build()
     m_func->m_headInstr->InsertAfter(m_func->m_tailInstr);
     m_func->m_isLeaf = true;  // until proven otherwise
 
+    if (m_func->GetJITFunctionBody()->IsParamAndBodyScopeMerged())
+    {
+        this->SetParamScopeDone();
+    }
+
     if (m_func->GetJITFunctionBody()->GetLocalClosureReg() != Js::Constants::NoRegister)
     {
         m_func->InitLocalClosureSyms();
@@ -3520,9 +3525,10 @@ IRBuilder::BuildElementSlotI1(Js::OpCode newOpcode, uint32 offset, Js::RegSlot r
     StackSym *   stackFuncPtrSym = nullptr;
     SymID        symID = m_func->GetJITFunctionBody()->GetLocalClosureReg();
     bool isLdSlotThatWasNotProfiled = false;
-    uint scopeSlotSize = m_func->GetJITFunctionBody()->GetScopeSlotArraySize();
     StackSym* closureSym = m_func->GetLocalClosureSym();
 
+    uint scopeSlotSize = this->IsParamScopeDone() ? m_func->GetJITFunctionBody()->GetScopeSlotArraySize() : m_func->GetJITFunctionBody()->GetParamScopeSlotArraySize();
+
     switch (newOpcode)
     {
         case Js::OpCode::LdParamSlot:
@@ -3532,7 +3538,7 @@ IRBuilder::BuildElementSlotI1(Js::OpCode newOpcode, uint32 offset, Js::RegSlot r
             // Fall through
 
         case Js::OpCode::LdLocalSlot:
-            if (PHASE_ON(Js::ClosureRangeCheckPhase, m_func))
+            if (!PHASE_OFF(Js::ClosureRangeCheckPhase, m_func))
             {
                 if ((uint32)slotId >= scopeSlotSize + Js::ScopeSlots::FirstSlotIndex)
                 {
@@ -3638,7 +3644,7 @@ IRBuilder::BuildElementSlotI1(Js::OpCode newOpcode, uint32 offset, Js::RegSlot r
 
         case Js::OpCode::StLocalSlot:
         case Js::OpCode::StLocalSlotChkUndecl:
-            if (PHASE_ON(Js::ClosureRangeCheckPhase, m_func))
+            if (!PHASE_OFF(Js::ClosureRangeCheckPhase, m_func))
             {
                 if ((uint32)slotId >= scopeSlotSize + Js::ScopeSlots::FirstSlotIndex)
                 {
@@ -6820,6 +6826,9 @@ IRBuilder::BuildEmpty(Js::OpCode newOpcode, uint32 offset)
         // This marks the end of a param socpe which is not merged with body scope.
         // So we have to first cache the closure so that we can use it to copy the initial values for
         // body syms from corresponding param syms (LdParamSlot). Body should get its own scope slot.
+        Assert(!this->IsParamScopeDone());
+        this->SetParamScopeDone();
+
         this->AddInstr(
             IR::Instr::New(
                 Js::OpCode::Ld_A,

lib/Backend/IRBuilder.h

@@ -71,6 +71,7 @@ class IRBuilder
         , m_switchBuilder(&m_switchAdapter)
         , m_stackFuncPtrSym(nullptr)
         , m_loopBodyForInEnumeratorArrayOpnd(nullptr)
+        , m_paramScopeDone(false)
 #if DBG
         , m_callsOnStack(0)
         , m_usedAsTemp(nullptr)
@@ -277,6 +278,9 @@ class IRBuilder
         return reg > 0 && reg < m_func->GetJITFunctionBody()->GetConstCount();
     }
 
+    bool                IsParamScopeDone() const { return m_paramScopeDone; }
+    void                SetParamScopeDone(bool done = true) { m_paramScopeDone = done; }
+
     Js::RegSlot         InnerScopeIndexToRegSlot(uint32) const;
     Js::RegSlot         GetEnvReg() const;
     Js::RegSlot         GetEnvRegForEvalCode() const;
@@ -348,6 +352,7 @@ class IRBuilder
     StackSym *          m_loopBodyRetIPSym;
     StackSym*           m_loopCounterSym;
     StackSym *          m_stackFuncPtrSym;
+    bool                m_paramScopeDone;
     bool                callTreeHasSomeProfileInfo;
     uint                finallyBlockLevel;
 

lib/Backend/Lower.cpp

@@ -8860,14 +8860,37 @@ Lowerer::LowerLdArrViewElem(IR::Instr * instr)
     IR::Instr * instrPrev = instr->m_prev;
 
     IR::RegOpnd * indexOpnd = instr->GetSrc1()->AsIndirOpnd()->GetIndexOpnd();
+    int32 offset = instr->GetSrc1()->AsIndirOpnd()->GetOffset();
 
     IR::Opnd * dst = instr->GetDst();
     IR::Opnd * src1 = instr->GetSrc1();
     IR::Opnd * src2 = instr->GetSrc2();
 
     IR::Instr * done;
 
-    if (indexOpnd || m_func->GetJITFunctionBody()->GetAsmJsInfo()->AccessNeedsBoundCheck((uint32)src1->AsIndirOpnd()->GetOffset()))
+    if (offset < 0)
+    {
+        IR::Opnd * oobValue = nullptr;
+        if(dst->IsFloat32())
+        {
+            oobValue = IR::MemRefOpnd::New(m_func->GetThreadContextInfo()->GetFloatNaNAddr(), TyFloat32, m_func);
+        }
+        else if(dst->IsFloat64())
+        {
+            oobValue = IR::MemRefOpnd::New(m_func->GetThreadContextInfo()->GetDoubleNaNAddr(), TyFloat64, m_func);
+        }
+        else
+        {
+            oobValue = IR::IntConstOpnd::New(0, dst->GetType(), m_func);
+        }
+        instr->ReplaceSrc1(oobValue);
+        if (src2)
+        {
+            instr->FreeSrc2();
+        }
+        return m_lowererMD.ChangeToAssign(instr);
+    }
+    if (indexOpnd || m_func->GetJITFunctionBody()->GetAsmJsInfo()->AccessNeedsBoundCheck((uint32)offset))
     {
         // CMP indexOpnd, src2(arrSize)
         // JA $helper
@@ -9145,6 +9168,7 @@ Lowerer::LowerStArrViewElem(IR::Instr * instr)
 
     // type of dst is the type of array
     IR::RegOpnd * indexOpnd = dst->AsIndirOpnd()->GetIndexOpnd();
+    int32 offset = dst->AsIndirOpnd()->GetOffset();
 
     Assert(!dst->IsFloat32() || src1->IsFloat32());
     Assert(!dst->IsFloat64() || src1->IsFloat64());
@@ -9156,7 +9180,12 @@ Lowerer::LowerStArrViewElem(IR::Instr * instr)
     {
         done = LowerWasmMemOp(instr, dst);
     }
-    else if (indexOpnd || m_func->GetJITFunctionBody()->GetAsmJsInfo()->AccessNeedsBoundCheck((uint32)dst->AsIndirOpnd()->GetOffset()))
+    else if (offset < 0)
+    {
+        instr->Remove();
+        return instrPrev;
+    }
+    else if (indexOpnd || m_func->GetJITFunctionBody()->GetAsmJsInfo()->AccessNeedsBoundCheck((uint32)offset))
     {
         // CMP indexOpnd, src2(arrSize)
         // JA $helper

lib/Backend/amd64/PeepsMD.cpp

@@ -41,6 +41,20 @@ PeepsMD::ProcessImplicitRegs(IR::Instr *instr)
             this->peeps->ClearReg(RegRDX);
         }
     }
+    else if (instr->m_opcode == Js::OpCode::XCHG)
+    {
+        // At time of writing, I believe that src1 is always identical to dst, but clear both for robustness.
+
+        // Either of XCHG's operands (but not both) can be a memory address, so only clear registers.
+        if (instr->GetSrc1()->IsRegOpnd())
+        {
+            this->peeps->ClearReg(instr->GetSrc1()->AsRegOpnd()->GetReg());
+        }
+        if (instr->GetSrc2()->IsRegOpnd())
+        {
+            this->peeps->ClearReg(instr->GetSrc2()->AsRegOpnd()->GetReg());
+        }
+    }
 }
 
 void

lib/Backend/i386/PeepsMD.cpp

@@ -47,6 +47,20 @@ PeepsMD::ProcessImplicitRegs(IR::Instr *instr)
             this->peeps->ClearReg(RegEDX);
         }
     }
+    else if (instr->m_opcode == Js::OpCode::XCHG)
+    {
+        // At time of writing, I believe that src1 is always identical to dst, but clear both for robustness.
+
+        // Either of XCHG's operands (but not both) can be a memory address, so only clear registers.
+        if (instr->GetSrc1()->IsRegOpnd())
+        {
+            this->peeps->ClearReg(instr->GetSrc1()->AsRegOpnd()->GetReg());
+        }
+        if (instr->GetSrc2()->IsRegOpnd())
+        {
+            this->peeps->ClearReg(instr->GetSrc2()->AsRegOpnd()->GetReg());
+        }
+    }
 }
 
 void

lib/Parser/Hash.cpp

@@ -131,18 +131,6 @@ void HashTbl::Grow()
 #endif
 }
 
-void HashTbl::ClearPidRefStacks()
-{
-    // Clear pidrefstack pointers from all existing pid's.
-    for (uint i = 0; i < m_luMask; i++)
-    {
-        for (IdentPtr pid = m_prgpidName[i]; pid; pid = pid->m_pidNext)
-        {
-            pid->m_pidRefStack = nullptr;
-        }
-    }
-}
-
 #if DEBUG
 uint HashTbl::CountAndVerifyItems(IdentPtr *buckets, uint bucketCount, uint mask)
 {

lib/Parser/Hash.h

@@ -393,7 +393,18 @@ class HashTbl
     NoReleaseAllocator* GetAllocator() {return &m_noReleaseAllocator;}
 
     bool Contains(_In_reads_(cch) LPCOLESTR prgch, int32 cch);
-    void ClearPidRefStacks();
+
+    template<typename Fn>
+    void VisitPids(Fn fn)
+    {
+        for (uint i = 0; i <= m_luMask; i++)
+        {
+            for (IdentPtr pid = m_prgpidName[i]; pid; pid = pid->m_pidNext)
+            {
+                fn(pid);
+            }
+        }
+    }
 
 private:
     NoReleaseAllocator m_noReleaseAllocator;            // to allocate identifiers