[MERGE #6279 @MikeHolman] September 2019 Security Update

Merge pull request #6279 from MikeHolman:servicing/1909 September 2019 Security Update that addresses the following issues in ChakraCore: CVE-2019-1138 CVE-2019-1217 CVE-2019-1237 CVE-2019-1298 CVE-2019-1300
chakra-core · Sep 10, 2019 · 7e9a2ee · 7e9a2ee
2 parents c5297b8 + edf5eee
commit 7e9a2ee
Show file tree

Hide file tree

Showing 17 changed files with 1,529 additions and 1,438 deletions.
diff --git a/.gitignore b/.gitignore
@@ -39,10 +39,7 @@ build_*.log
 build_*.wrn
 Build/ipch/
 Build/swum-cache.txt
-Build/VCBuild.Lite/
-Build/VCBuild.NoJIT/
-Build/VCBuild.SWB/
-Build/VCBuild/
+Build/VCBuild*/
 buildchk.*
 buildfre.*
 out/

diff --git a/Build/NuGet/.pack-version b/Build/NuGet/.pack-version
@@ -1 +1 @@
-1.11.12
+1.11.13
diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp
@@ -5422,7 +5422,14 @@ BackwardPass::TrackObjTypeSpecProperties(IR::PropertySymOpnd *opnd, BasicBlock *
                         // Some instr protected by this one requires a monomorphic type check. (E.g., final type opt,
                         // fixed field not loaded from prototype.) Note the IsTypeAvailable test above: only do this at
                         // the initial type check that protects this path.
-                        opnd->SetMonoGuardType(bucket->GetMonoGuardType());
+                        if (!opnd->SetMonoGuardType(bucket->GetMonoGuardType()))
+                        {
+                            // We can't safely check for the required type here. Clear the objtypespec info to disable optimization
+                            // using this inline cache, since there appears to be a mismatch, and re-jit.
+                            // (Dead store pass is too late to generate the bailout points we need to use this type correctly.)
+                            this->currentInstr->m_func->ClearObjTypeSpecFldInfo(opnd->m_inlineCacheIndex);
+                            throw Js::RejitException(RejitReason::FailedEquivalentTypeCheck);
+                        }
                         this->currentInstr->ChangeEquivalentToMonoTypeCheckBailOut();
                     }
                     bucket->SetMonoGuardType(nullptr);
@@ -8742,7 +8749,7 @@ BackwardPass::RestoreInductionVariableValuesAfterMemOp(Loop *loop)
         StackSym *sym = localFunc->m_symTable->FindStackSym(symId)->GetInt32EquivSym(localFunc);
 
         IR::Opnd *inductionVariableOpnd = IR::RegOpnd::New(sym, IRType::TyInt32, localFunc);
-        IR::Opnd *sizeOpnd = globOpt->GenerateInductionVariableChangeForMemOp(loop, inductionVariableChangeInfo.unroll);
+        IR::Opnd *sizeOpnd = globOpt->GenerateInductionVariableChangeForMemOp(loop, inductionVariableChangeInfo.unroll, loop->memOpInfo->instr);
         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

diff --git a/lib/Backend/Func.cpp b/lib/Backend/Func.cpp
@@ -348,6 +348,9 @@ Func::Codegen(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
             case RejitReason::MemOpDisabled:
                 outputData->disableMemOp = TRUE;
                 break;
+            case RejitReason::FailedEquivalentTypeCheck:
+                // No disable flag. The thrower of the re-jit exception must guarantee that objtypespec is disabled where appropriate.
+                break;
             default:
                 Assume(UNREACHED);
             }
@@ -1521,6 +1524,12 @@ Func::GetObjTypeSpecFldInfo(const uint index) const
     return GetWorkItem()->GetJITTimeInfo()->GetObjTypeSpecFldInfo(index);
 }
 
+void
+Func::ClearObjTypeSpecFldInfo(const uint index)
+{
+    GetWorkItem()->GetJITTimeInfo()->ClearObjTypeSpecFldInfo(index);
+}
+
 ObjTypeSpecFldInfo*
 Func::GetGlobalObjTypeSpecFldInfo(uint propertyInfoId) const
 {

diff --git a/lib/Backend/Func.h b/lib/Backend/Func.h
@@ -579,6 +579,7 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
     Js::Var AllocateNumber(double value);
 
     ObjTypeSpecFldInfo* GetObjTypeSpecFldInfo(const uint index) const;
+    void ClearObjTypeSpecFldInfo(const uint index);
     ObjTypeSpecFldInfo* GetGlobalObjTypeSpecFldInfo(uint propertyInfoId) const;
 
     // Gets an inline cache pointer to use in jitted code. Cached data may not be stable while jitting. Does not return null.

diff --git a/lib/Backend/FunctionJITTimeInfo.cpp b/lib/Backend/FunctionJITTimeInfo.cpp
@@ -311,6 +311,18 @@ FunctionJITTimeInfo::GetObjTypeSpecFldInfo(uint index) const
     return reinterpret_cast<ObjTypeSpecFldInfo *>(m_data.objTypeSpecFldInfoArray[index]);
 }
 
+void
+FunctionJITTimeInfo::ClearObjTypeSpecFldInfo(uint index)
+{
+    if (m_data.objTypeSpecFldInfoArray == nullptr)
+    {
+        return;
+    }
+    AssertOrFailFast(index < m_data.objTypeSpecFldInfoCount);
+
+    m_data.objTypeSpecFldInfoArray[index] = nullptr;
+}
+
 ObjTypeSpecFldInfo *
 FunctionJITTimeInfo::GetGlobalObjTypeSpecFldInfo(uint index) const
 {

diff --git a/lib/Backend/FunctionJITTimeInfo.h b/lib/Backend/FunctionJITTimeInfo.h
@@ -38,6 +38,7 @@ class FunctionJITTimeInfo
     const BVFixed * GetInlineesBV() const;
     const FunctionJITTimeInfo * GetJitTimeDataFromFunctionInfoAddr(intptr_t polyFuncInfo) const;
     ObjTypeSpecFldInfo * GetObjTypeSpecFldInfo(uint index) const;
+    void ClearObjTypeSpecFldInfo(uint index);
     ObjTypeSpecFldInfo * GetGlobalObjTypeSpecFldInfo(uint index) const;
     uint GetGlobalObjTypeSpecFldInfoCount() const;
     const FunctionJITRuntimeInfo * GetInlineeForTargetInlineeRuntimeData(const Js::ProfileId profiledCallSiteId, intptr_t inlineeFuncBodyAddr) const;

diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp
@@ -1167,6 +1167,10 @@ void GlobOpt::InsertValueCompensation(
     IR::Instr *insertBeforeInstr = predecessor->GetLastInstr();
     Func *const func = insertBeforeInstr->m_func;
     bool setLastInstrInPredecessor;
+    // If this is a loop back edge, and the successor has been completed, don't attempt to update its block data.
+    // The update is unnecessary, and the data has likely been freed.
+    bool updateSuccessorBlockData = !this->isPerformingLoopBackEdgeCompensation || successor->GetDataUseCount() > 0;
+
     if(insertBeforeInstr->IsBranchInstr() || insertBeforeInstr->m_opcode == Js::OpCode::BailTarget)
     {
         // Don't insert code between the branch and the corresponding ByteCodeUses instructions
@@ -1257,29 +1261,33 @@ void GlobOpt::InsertValueCompensation(
             // Merge the head segment length value
             Assert(predecessorBlockData.liveVarSyms->Test(predecessorHeadSegmentLengthSym->m_id));
             predecessorBlockData.liveVarSyms->Set(mergedHeadSegmentLengthSym->m_id);
-            successorBlockData.liveVarSyms->Set(mergedHeadSegmentLengthSym->m_id);
             Value *const predecessorHeadSegmentLengthValue =
                 predecessorBlockData.FindValue(predecessorHeadSegmentLengthSym);
             Assert(predecessorHeadSegmentLengthValue);
             predecessorBlockData.SetValue(predecessorHeadSegmentLengthValue, mergedHeadSegmentLengthSym);
-            Value *const mergedHeadSegmentLengthValue = successorBlockData.FindValue(mergedHeadSegmentLengthSym);
-            if(mergedHeadSegmentLengthValue)
+
+            if (updateSuccessorBlockData)
             {
-                Assert(mergedHeadSegmentLengthValue->GetValueNumber() != predecessorHeadSegmentLengthValue->GetValueNumber());
-                if(predecessorHeadSegmentLengthValue->GetValueInfo() != mergedHeadSegmentLengthValue->GetValueInfo())
+                successorBlockData.liveVarSyms->Set(mergedHeadSegmentLengthSym->m_id);
+                Value *const mergedHeadSegmentLengthValue = successorBlockData.FindValue(mergedHeadSegmentLengthSym);
+                if(mergedHeadSegmentLengthValue)
                 {
-                    mergedHeadSegmentLengthValue->SetValueInfo(
-                        ValueInfo::MergeLikelyIntValueInfo(
-                            this->alloc,
-                            mergedHeadSegmentLengthValue,
-                            predecessorHeadSegmentLengthValue,
-                            mergedHeadSegmentLengthValue->GetValueInfo()->Type()
-                                .Merge(predecessorHeadSegmentLengthValue->GetValueInfo()->Type())));
+                    Assert(mergedHeadSegmentLengthValue->GetValueNumber() != predecessorHeadSegmentLengthValue->GetValueNumber());
+                    if(predecessorHeadSegmentLengthValue->GetValueInfo() != mergedHeadSegmentLengthValue->GetValueInfo())
+                    {
+                        mergedHeadSegmentLengthValue->SetValueInfo(
+                            ValueInfo::MergeLikelyIntValueInfo(
+                                this->alloc,
+                                mergedHeadSegmentLengthValue,
+                                predecessorHeadSegmentLengthValue,
+                                mergedHeadSegmentLengthValue->GetValueInfo()->Type()
+                                    .Merge(predecessorHeadSegmentLengthValue->GetValueInfo()->Type())));
+                    }
+                }
+                else
+                {
+                    successorBlockData.SetValue(CopyValue(predecessorHeadSegmentLengthValue), mergedHeadSegmentLengthSym);
                 }
-            }
-            else
-            {
-                successorBlockData.SetValue(CopyValue(predecessorHeadSegmentLengthValue), mergedHeadSegmentLengthSym);
             }
         }
 
@@ -1300,27 +1308,31 @@ void GlobOpt::InsertValueCompensation(
             // Merge the length value
             Assert(predecessorBlockData.liveVarSyms->Test(predecessorLengthSym->m_id));
             predecessorBlockData.liveVarSyms->Set(mergedLengthSym->m_id);
-            successorBlockData.liveVarSyms->Set(mergedLengthSym->m_id);
             Value *const predecessorLengthValue = predecessorBlockData.FindValue(predecessorLengthSym);
             Assert(predecessorLengthValue);
             predecessorBlockData.SetValue(predecessorLengthValue, mergedLengthSym);
-            Value *const mergedLengthValue = successorBlockData.FindValue(mergedLengthSym);
-            if(mergedLengthValue)
+
+            if (updateSuccessorBlockData)
             {
-                Assert(mergedLengthValue->GetValueNumber() != predecessorLengthValue->GetValueNumber());
-                if(predecessorLengthValue->GetValueInfo() != mergedLengthValue->GetValueInfo())
+                successorBlockData.liveVarSyms->Set(mergedLengthSym->m_id);
+                Value *const mergedLengthValue = successorBlockData.FindValue(mergedLengthSym);
+                if(mergedLengthValue)
                 {
-                    mergedLengthValue->SetValueInfo(
-                        ValueInfo::MergeLikelyIntValueInfo(
-                            this->alloc,
-                            mergedLengthValue,
-                            predecessorLengthValue,
-                            mergedLengthValue->GetValueInfo()->Type().Merge(predecessorLengthValue->GetValueInfo()->Type())));
+                    Assert(mergedLengthValue->GetValueNumber() != predecessorLengthValue->GetValueNumber());
+                    if(predecessorLengthValue->GetValueInfo() != mergedLengthValue->GetValueInfo())
+                    {
+                        mergedLengthValue->SetValueInfo(
+                            ValueInfo::MergeLikelyIntValueInfo(
+                                this->alloc,
+                                mergedLengthValue,
+                                predecessorLengthValue,
+                                mergedLengthValue->GetValueInfo()->Type().Merge(predecessorLengthValue->GetValueInfo()->Type())));
+                    }
+                }
+                else
+                {
+                    successorBlockData.SetValue(CopyValue(predecessorLengthValue), mergedLengthSym);
                 }
-            }
-            else
-            {
-                successorBlockData.SetValue(CopyValue(predecessorLengthValue), mergedLengthSym);
             }
         }
 
@@ -2087,6 +2099,7 @@ bool GlobOpt::CollectMemcopyStElementI(IR::Instr *instr, Loop *loop)
 
     // Consider: Can we remove the count field?
     memcopyInfo->count++;
+    AssertOrFailFast(memcopyInfo->count <= 1);
     memcopyInfo->base = baseSymID;
 
     return true;
@@ -2226,7 +2239,14 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Va
                 {
                     Loop::InductionVariableChangeInfo inductionVariableChangeInfo = { 0, 0 };
                     inductionVariableChangeInfo = loop->memOpInfo->inductionVariableChangeInfoMap->Lookup(inductionSymID, inductionVariableChangeInfo);
-                    inductionVariableChangeInfo.unroll++;
+
+                    // If inductionVariableChangeInfo.unroll has been invalidated, do
+                    // not modify the Js::Constants::InvalidLoopUnrollFactor value
+                    if (inductionVariableChangeInfo.unroll != Js::Constants::InvalidLoopUnrollFactor)
+                    {
+                        inductionVariableChangeInfo.unroll++;
+                    }
+
                     inductionVariableChangeInfo.isIncremental = isIncr;
                     loop->memOpInfo->inductionVariableChangeInfoMap->Item(inductionSymID, inductionVariableChangeInfo);
                 }
@@ -16677,6 +16697,7 @@ GlobOpt::GetOrGenerateLoopCountForMemOp(Loop *loop)
 IR::Opnd *
 GlobOpt::GenerateInductionVariableChangeForMemOp(Loop *loop, byte unroll, IR::Instr *insertBeforeInstr)
 {
+    AssertOrFailFast(unroll != Js::Constants::InvalidLoopUnrollFactor);
     LoopCount *loopCount = loop->loopCount;
     IR::Opnd *sizeOpnd = nullptr;
     Assert(loopCount);
@@ -16714,11 +16735,12 @@ GlobOpt::GenerateInductionVariableChangeForMemOp(Loop *loop, byte unroll, IR::In
 
             IR::Opnd *unrollOpnd = IR::IntConstOpnd::New(unroll, type, localFunc);
 
-            InsertInstr(IR::Instr::New(Js::OpCode::Mul_I4,
-                sizeOpnd,
-                loopCountOpnd,
-                unrollOpnd,
-                localFunc));
+            IR::Instr* inductionChangeMultiplier = IR::Instr::New(
+                Js::OpCode::Mul_I4, sizeOpnd, loopCountOpnd, unrollOpnd, localFunc);
+
+            InsertInstr(inductionChangeMultiplier);
+
+            inductionChangeMultiplier->ConvertToBailOutInstr(loop->bailOutInfo, IR::BailOutOnOverflow);
 
         }
     }

diff --git a/lib/Backend/Opnd.h b/lib/Backend/Opnd.h
@@ -799,9 +799,17 @@ class PropertySymOpnd sealed : public SymOpnd
         return this->monoGuardType;
     }
 
-    void SetMonoGuardType(JITTypeHolder type)
+    bool SetMonoGuardType(JITTypeHolder type)
     {
+        if (!(this->monoGuardType == nullptr || this->monoGuardType == type) ||
+            !((HasEquivalentTypeSet() && GetEquivalentTypeSet()->Contains(type)) ||
+              (!HasEquivalentTypeSet() && GetType() == type)))
+        {
+            // Required type is not in the available set, or we already set the type to something else. Inform the caller.
+            return false;
+        }
         this->monoGuardType = type;
+        return true;
     }
 
     bool NeedsMonoCheck() const

diff --git a/lib/Common/ChakraCoreVersion.h b/lib/Common/ChakraCoreVersion.h
@@ -17,7 +17,7 @@
 // ChakraCore version number definitions (used in ChakraCore binary metadata)
 #define CHAKRA_CORE_MAJOR_VERSION 1
 #define CHAKRA_CORE_MINOR_VERSION 11
-#define CHAKRA_CORE_PATCH_VERSION 12
+#define CHAKRA_CORE_PATCH_VERSION 13
 #define CHAKRA_CORE_VERSION_RELEASE_QFE 0 // Redundant with PATCH_VERSION. Keep this value set to 0.
 
 // -------------

diff --git a/lib/Runtime/ByteCode/ByteCodeCacheReleaseFileVersion.h b/lib/Runtime/ByteCode/ByteCodeCacheReleaseFileVersion.h
@@ -5,6 +5,6 @@
 // NOTE: If there is a merge conflict the correct fix is to make a new GUID.
 // This file was generated with tools\update_bytecode_version.ps1
 
-// {3096A219-129D-4A4A-A61C-186D03BB25B7}
+// {81AEEA4B-AE4E-40C0-848F-6DB7C5F49F55}
 const GUID byteCodeCacheReleaseFileVersion =
-{ 0x3096a219, 0x129d, 0x4a4a, { 0xa6, 0x1c, 0x18, 0x6d, 0x3, 0xbb, 0x25, 0xb7 } };
+{ 0x81AEEA4B, 0xAE4E, 0x40C0, { 0x84, 0x8F, 0x6D, 0xB7, 0xC5, 0xF4, 0x9F, 0x55 } };
diff --git a/lib/Runtime/Library/BoundFunction.cpp b/lib/Runtime/Library/BoundFunction.cpp
@@ -354,6 +354,12 @@ namespace Js
             Var varLength;
             if (targetFunction->GetProperty(targetFunction, PropertyIds::length, &varLength, nullptr, requestContext))
             {
+                if (!TaggedInt::Is(varLength))
+                {
+                    // ToInt32 conversion on non-primitive length can invalidate assumptions made by the JIT,
+                    // so add implicit call flag if length isn't a TaggedInt already
+                    requestContext->GetThreadContext()->AddImplicitCallFlags(ImplicitCall_Accessor);
+                }
                 len = JavascriptConversion::ToInt32(varLength, requestContext);
             }
 

diff --git a/lib/Runtime/Library/JsBuiltIn/JsBuiltIn.js b/lib/Runtime/Library/JsBuiltIn/JsBuiltIn.js
@@ -8,19 +8,19 @@
 (function (intrinsic) {
     var platform = intrinsic.JsBuiltIn;
 
-    let FunctionsEnum = {
-        ArrayValues: { className: "Array", methodName: "values", argumentsCount: 0, forceInline: true /*optional*/, alias: "Symbol.iterator" },
-        ArrayKeys: { className: "Array", methodName: "keys", argumentsCount: 0, forceInline: true /*optional*/ },
-        ArrayEntries: { className: "Array", methodName: "entries", argumentsCount: 0, forceInline: true /*optional*/ },
-        ArrayIndexOf: { className: "Array", methodName: "indexOf", argumentsCount: 1, forceInline: true /*optional*/ },
-        ArrayFilter: { className: "Array", methodName: "filter", argumentsCount: 1, forceInline: true /*optional*/ },
-    };
-
     var setPrototype = platform.builtInSetPrototype;
     var _objectDefineProperty = platform.builtInJavascriptObjectEntryDefineProperty;
     var Symbol = platform.Symbol;
     var CreateObject = platform.builtInJavascriptObjectCreate;
 
+    let FunctionsEnum = {
+        ArrayValues: setPrototype({ className: "Array", methodName: "values", argumentsCount: 0, forceInline: true /*optional*/, alias: "Symbol.iterator" }, null),
+        ArrayKeys: setPrototype({ className: "Array", methodName: "keys", argumentsCount: 0, forceInline: true /*optional*/ }, null),
+        ArrayEntries: setPrototype({ className: "Array", methodName: "entries", argumentsCount: 0, forceInline: true /*optional*/ }, null),
+        ArrayIndexOf: setPrototype({ className: "Array", methodName: "indexOf", argumentsCount: 1, forceInline: true /*optional*/ }, null),
+        ArrayFilter: setPrototype({ className: "Array", methodName: "filter", argumentsCount: 1, forceInline: true /*optional*/ }, null),
+    };
+
     platform.registerChakraLibraryFunction("ArrayIterator", function (arrayObj, iterationKind) {
         "use strict";
         __chakraLibrary.InitInternalProperties(this, 4, "__$arrayObj$__", "__$nextIndex$__", "__$kind$__", "__$internalDone$__");