[1.6>1.7] [MERGE #3509 @pleath] 17-08 ChakraCore servicing release

Merge pull request #3509 from pleath:1708

Addresses the following:

CVE-2017-0228
CVE-2017-8634
CVE-2017-8635
CVE-2017-8636
CVE-2017-8637
CVE-2017-8638
CVE-2017-8640
CVE-2017-8641
CVE-2017-8645
CVE-2017-8646
CVE-2017-8647
CVE-2017-8655
CVE-2017-8656
CVE-2017-8657
CVE-2017-8658
CVE-2017-8659
CVE-2017-8670
CVE-2017-8671
CVE-2017-8672
CVE-2017-8674

Details:

[CVE-2017-8656] PreVisitCatch doesn't call SetIsCatch for all cases

The catch block has a var declaration with the same name as the destructured parameter. When we emit code for initializing vars we don't emit the initialization code for the symbol in the body as the catch param is not marked. The fix is to mark the destructured param as catch param.

Blind constants more aggressively in jitted code

Let RPC do handle marshalling for the JIT process

[CVE-2017-8647] Fix oop jit diff to check msvcrt instead of ucrt

[CVE-2017-8637] [CVE-2017-8659] Harden JIT process against bad input

[CVE-2017-8655] Prevent restoring inlinee frame with the wrong function object

Invalidate lastUsedSegment of an array at the right point

[CVE-2017-8638] Fix function object inline cache if function body got reparsed

[CVE-2017-8657] Postpone adding deferred top-level functions to the deferred function dictionary. Do this so that if byte code gen is interrupted or restarted, we are not left with orphaned functions in the dictionary that may be semi-initialized.

[CVE-2017-8658] Don't attempt to use deferred function stubs inside a formal argument list, as this leads to issues with arrow functions, nested functions, etc.

[CVE-2017-8635] Don't try to reuse property indexes on re-add of a deleted property if the object is non-extensible. Doing so exhausts the free indexes on repeated re-adds, which violates our assumption that a free index will always be available if a property has been deleted.

[CVE-2017-8671] Consider CallFlags_ExtraArg in Function.call implementation. Extra arg (for proxy, etc.) should not be a factor when copying/eliding arguments to delegate to the real target.

[CVE-2017-8674] Re-map function object types if AutoRestoreFunctionInfo has to restore FunctionInfo on failed re-parse/byte code gen.

[CVE-2017-8640] Mark the correct function node when we're detecting 'arguments' declaration that overrides the built-in declaration.

[CVE-2017-8670] Make sure we mark the correct function node when we're processing formal named 'arguments' in a destructured expression.

[CVE-2017-8672] Avoid passing JsNull as JsFunction*

In JavascriptStackWalker GetCaller we were sending out JsNull as a
JavascriptFunction* in the outparam. I checked all locations that
did a call to this function, and recursively as far as the outparam
escaped. Virtually all of the callers of the function or the other
functions that allowed escape of the outparam were safe, as checks
against the return value of GetCaller are near-ubiquitous. The one
callsite where this wasn't the case was in GlobalObject.cpp L#693,
which is in VEval. This location used the outparam without checks,
which meant that if a JsNull object were returned, we'd call other
functions which wouldn't necessarily be correct.

[CVE-2017-8645] Whitelist the type of functions allow to be asm.js module

[CVE-2017-8646] Correctly use PushPopFrameHelper in ProcessLinkFailedAsmJsModule.
No longer ignore arbitrary javascript code after a `let`

[CVE-2017-8641] Integer overflow in chakra!Js::GlobalObject
Integer overflow can happen in multiple cases of add-mult and allocate.
Fix by using UInt32Math::AddMul() to catch overflows.

[CVE-2017-8634] Type confusion in Concat due to IsConcatSpreadable

[CVE-2017-8636] Limiting args count to a certain limit. This limit will be enforced during the parser time. During the bytecode time we add so many extra args which actually overflowed the initial assumption.

[CVE-2017-0228]  We have attempted to fix the ReverseHelper case where the inline segment was swapped from head to bottom due to reverse loop below. However that fix was done for multi-segment scenario. The EnsureHeadStartsFromZero can also insert head segment in between if the segment's length is less than Array's length (look at the loop above), which will make the code vulnerable. Fixed that scenario by allocating the segment (for the single segment case) if the segment's length is less than array's length.

Author: pleath

Build/Chakra.Build.props

@@ -4,6 +4,7 @@
   <PropertyGroup>
     <Win32_WinNTVersion Condition="'$(NtTargetVersion)'=='$(NtTargetVersion_Win7)'">0x0601</Win32_WinNTVersion>
     <Win32_WinNTVersion Condition="'$(NtTargetVersion)'=='$(NtTargetVersion_Win8)'">0x0602</Win32_WinNTVersion>
+    <Win32_WinNTVersion Condition="'$(NtTargetVersion)'=='$(NtTargetVersion_WinBlue)'">0x0603</Win32_WinNTVersion>
     <Win32_WinNTVersion Condition="'$(NtTargetVersion)'=='$(NtTargetVersion_Win10)'">0x0A00</Win32_WinNTVersion>
 
     <!-- Always use Platform SDK for core builds -->

Build/Common.Build.Default.props

@@ -4,6 +4,7 @@
   <PropertyGroup>
     <NtTargetVersion_Win7 >0x601</NtTargetVersion_Win7>
     <NtTargetVersion_Win8 >0x602</NtTargetVersion_Win8>
+    <NtTargetVersion_WinBlue>0x603</NtTargetVersion_WinBlue>
     <NtTargetVersion_Win10>0xA00</NtTargetVersion_Win10>
   </PropertyGroup>
 

Build/Common.Build.props

@@ -29,6 +29,8 @@
       <!-- ======== sources.inc ======== -->
       <!-- generates SAL annotations for our interface -->
       <AdditionalOptions>%(AdditionalOptions) -sal_local</AdditionalOptions>
+      
+      <PreprocessorDefinitions>%(PreprocessorDefinitions);WINVER=$(Win32_WinNTVersion)</PreprocessorDefinitions>
     </Midl>
     <ClCompile>
       <PreprocessorDefinitions>%(PreprocessorDefinitions);NOMINMAX;USE_EDGEMODE_JSRT</PreprocessorDefinitions>
@@ -105,6 +107,7 @@
 
       <MinimumRequiredVersion Condition="'$(NtTargetVersion)'=='$(NtTargetVersion_Win7)'" >6.1</MinimumRequiredVersion>
       <MinimumRequiredVersion Condition="'$(NtTargetVersion)'=='$(NtTargetVersion_Win8)'" >6.2</MinimumRequiredVersion>
+      <MinimumRequiredVersion Condition="'$(NtTargetVersion)'=='$(NtTargetVersion_WinBlue)'" >6.3</MinimumRequiredVersion>
       <MinimumRequiredVersion Condition="'$(NtTargetVersion)'=='$(NtTargetVersion_Win10)'" >10.00</MinimumRequiredVersion>
 
       <!-- Always set the checksum -->

lib/Backend/AsmJsJITInfo.cpp

@@ -60,7 +60,7 @@ AsmJsJITInfo::GetArgTypeArray() const
 Js::AsmJsVarType::Which
 AsmJsJITInfo::GetArgType(Js::ArgSlot argNum) const
 {
-    Assert(argNum < GetArgCount());
+    AssertOrFailFast(argNum < GetArgCount());
     return GetArgTypeArray()[argNum];
 }
 

lib/Backend/AsmJsJITInfo.h

@@ -28,7 +28,6 @@ class AsmJsJITInfo
     Js::ArgSlot GetArgCount() const;
     Js::ArgSlot GetArgByteSize() const;
     Js::AsmJsRetType::Which GetRetType() const;
-    Js::AsmJsVarType::Which * GetArgTypeArray() const;
     Js::AsmJsVarType::Which GetArgType(Js::ArgSlot argNum) const;
 
 #ifdef ENABLE_WASM
@@ -40,6 +39,7 @@ class AsmJsJITInfo
     bool AccessNeedsBoundCheck(uint offset) const;
 
 private:
+    Js::AsmJsVarType::Which * GetArgTypeArray() const;
     AsmJsDataIDL m_data;
 #endif
 };

lib/Backend/CodeGenWorkItem.h

@@ -58,6 +58,13 @@ struct CodeGenWorkItem : public JsUtil::Job
         return functionBody->GetScriptContext();
     }
 
+    uint GetByteCodeLength() const
+    {
+        return this->functionBody->IsInDebugMode()
+            ? this->functionBody->GetOriginalByteCode()->GetLength()
+            : this->functionBody->GetByteCode()->GetLength();
+    }
+
     Js::FunctionBody* GetFunctionBody() const
     {
         return functionBody;

lib/Backend/Func.cpp

@@ -153,6 +153,14 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
 
     Assert(this->IsInlined() == !!runtimeInfo);
 
+    AssertOrFailFast(!HasProfileInfo() || GetReadOnlyProfileInfo()->GetLoopCount() == GetJITFunctionBody()->GetLoopCount());
+    Js::RegSlot tmpResult;
+    AssertOrFailFast(!UInt32Math::Add(GetJITFunctionBody()->GetConstCount(), GetJITFunctionBody()->GetVarCount(), &tmpResult));
+    AssertOrFailFast(GetJITFunctionBody()->IsAsmJsMode() || GetJITFunctionBody()->GetFirstTmpReg() <= GetJITFunctionBody()->GetLocalsCount());
+    AssertOrFailFast(!IsLoopBody() || m_workItem->GetLoopNumber() < GetJITFunctionBody()->GetLoopCount());
+    AssertOrFailFast(CONFIG_FLAG(Prejit) || CONFIG_ISENABLED(Js::ForceNativeFlag) || GetJITFunctionBody()->GetByteCodeLength() < (uint)CONFIG_FLAG(MaxJITFunctionBytecodeByteLength));
+    GetJITFunctionBody()->EnsureConsistentConstCount();
+
     if (this->IsTopFunc())
     {
         outputData->hasJittedStackClosure = false;

lib/Backend/IR.cpp

@@ -84,6 +84,12 @@ Instr::HasEquivalentTypeCheckBailOut() const
     return this->HasBailOutInfo() && IR::IsEquivalentTypeCheckBailOutKind(this->GetBailOutKind());
 }
 
+bool
+Instr::HasBailOnNoProfile() const
+{
+    return this->HasBailOutInfo() && this->GetBailOutKind() == IR::BailOutOnNoProfile;
+}
+
 void
 Instr::ChangeEquivalentToMonoTypeCheckBailOut()
 {

lib/Backend/IR.h

@@ -203,6 +203,7 @@ class Instr
     bool            HasAuxBailOut() const { return hasAuxBailOut; }
     bool            HasTypeCheckBailOut() const;
     bool            HasEquivalentTypeCheckBailOut() const;
+    bool            HasBailOnNoProfile() const;
     void            ClearBailOutInfo();
     bool            IsDstNotAlwaysConvertedToInt32() const;
     bool            IsDstNotAlwaysConvertedToNumber() const;

lib/Backend/IRBuilder.cpp

@@ -400,7 +400,7 @@ IRBuilder::Build()
     Js::RegSlot tempCount = m_func->GetJITFunctionBody()->GetTempCount();
     if (tempCount > 0)
     {
-        this->tempMap = (SymID*)m_tempAlloc->AllocZero(sizeof(SymID) * tempCount);
+        this->tempMap = AnewArrayZ(m_tempAlloc, SymID, tempCount);
         this->fbvTempUsed = BVFixed::New<JitArenaAllocator>(tempCount, m_tempAlloc);
     }
     else
@@ -446,6 +446,7 @@ IRBuilder::Build()
 #endif
 
         lastOffset = m_func->m_workItem->GetLoopHeader()->endOffset;
+        AssertOrFailFast(lastOffset < m_func->GetJITFunctionBody()->GetByteCodeLength());
         // Ret is created at lastOffset + 1, so we need lastOffset + 2 entries
         offsetToInstructionCount = lastOffset + 2;
 
@@ -2721,6 +2722,7 @@ IRBuilder::BuildUnsigned1(Js::OpCode newOpcode, uint32 offset, uint32 num)
 
         case Js::OpCode::ProfiledLoopStart:
         {
+            AssertOrFailFast(num < m_func->GetJITFunctionBody()->GetLoopCount());
             // If we're in profiling SimpleJit and jitting loop bodies, we need to keep this until lowering.
             if (m_func->DoSimpleJitDynamicProfile() && m_func->GetJITFunctionBody()->DoJITLoopBody())
             {
@@ -2771,6 +2773,7 @@ IRBuilder::BuildUnsigned1(Js::OpCode newOpcode, uint32 offset, uint32 num)
 
         case Js::OpCode::ProfiledLoopEnd:
         {
+            AssertOrFailFast(num < m_func->GetJITFunctionBody()->GetLoopCount());
             // TODO: Decide whether we want the implicit loop call flags to be recorded in simplejitted loop bodies
             if (m_func->DoSimpleJitDynamicProfile() && m_func->GetJITFunctionBody()->DoJITLoopBody())
             {