Skip to content

Commit 8bf433a

Browse files
author
Suwei Chen
committed
[1.7>master] [MERGE chakra-core#3729 @suwc] 17-09 ChakraCore servicing release
Merge pull request chakra-core#3729 from suwc:build/suwc/1709B_1.7 [CVE-2017-8741]: Limit JSON Stringify Loop to Initialized Portion [CVE-2017-8748] Fix UAF caused by GC during bailout [CVE-2017-11767] Do not instantiate param scope if only the function expression symbol is captured [CVE-2017-8756] JIT peephole optimization error [CVE-2017-8753] Array Reverse OOM RCE [CVE-2017-8729] incorrect object pattern. [CVE-2017-8739] buffer overread IsMissingItem. [CVE-2017-8751]Type confusion casting undefined with TypeOfPrototypeObjectDictionary type [CVE-2017-8757]RCE on Windows Insider Preview [CVE-2017-11764]Parser::ParseCatch doesn't handle "eval" [CVE-2017-8660] Uninitialized local variables [CVE-2017-8755] Fail fast if we can't reparse asm.js module after linking failure [CVE-2017-8649] Bytecode tempering mitigation code accidently turned off - Internal [CVE-2017-8740] Fix bad byte code gen for 'with'. [CVE-2017-8752]fix missing bound check in asm.js in case of constant negative index
2 parents 027c7a8 + 0709816 commit 8bf433a

32 files changed

Lines changed: 311 additions & 102 deletions

‎lib/Backend/BailOut.cpp‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -865,12 +865,12 @@ BailOutRecord::RestoreValue(IR::BailOutKind bailOutKind, Js::JavascriptCallStack
865865
{
866866
// Register save space (offset is the register number and index into the register save space)
867867
// Index is one based, so subtract one
868-
Js::Var * registerSaveSpace = registerSaves ? registerSaves : (Js::Var *)scriptContext->GetThreadContext()->GetBailOutRegisterSaveSpace();
868+
AssertOrFailFast(registerSaves);
869869

870870
if (isFloat64)
871871
{
872872
Assert(RegTypes[LinearScanMD::GetRegisterFromSaveIndex(offset)] == TyFloat64);
873-
dblValue = *((double*)&(registerSaveSpace[offset - 1]));
873+
dblValue = *((double*)&(registerSaves[offset - 1]));
874874
#ifdef _M_ARM
875875
BAILOUT_VERBOSE_TRACE(newInstance->function->GetFunctionBody(), bailOutKind, _u("Register %-4S %4d"), RegNames[(offset - RegD0) / 2 + RegD0], offset);
876876
#else
@@ -885,17 +885,17 @@ BailOutRecord::RestoreValue(IR::BailOutKind bailOutKind, Js::JavascriptCallStack
885885
isSimd128B8 || isSimd128B16
886886
)
887887
{
888-
simdValue = *((SIMDValue *)&(registerSaveSpace[offset - 1]));
888+
simdValue = *((SIMDValue *)&(registerSaves[offset - 1]));
889889
}
890890
else if (isInt32)
891891
{
892892
Assert(RegTypes[LinearScanMD::GetRegisterFromSaveIndex(offset)] != TyFloat64);
893-
int32Value = ::Math::PointerCastToIntegralTruncate<int32>(registerSaveSpace[offset - 1]);
893+
int32Value = ::Math::PointerCastToIntegralTruncate<int32>(registerSaves[offset - 1]);
894894
}
895895
else
896896
{
897897
Assert(RegTypes[LinearScanMD::GetRegisterFromSaveIndex(offset)] != TyFloat64);
898-
value = registerSaveSpace[offset - 1];
898+
value = registerSaves[offset - 1];
899899
}
900900

901901
BAILOUT_VERBOSE_TRACE(newInstance->function->GetFunctionBody(), bailOutKind, _u("Register %-4S %4d"), RegNames[LinearScanMD::GetRegisterFromSaveIndex(offset)], offset);
@@ -1171,11 +1171,19 @@ uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::Imp
11711171
// Do not remove the following code.
11721172
// Need to capture the int registers on stack as threadContext->bailOutRegisterSaveSpace is allocated from ThreadAlloc and is not scanned by recycler.
11731173
// We don't want to save float (xmm) registers as they can be huge and they cannot contain a var.
1174-
Js::Var registerSaves[INT_REG_COUNT];
1174+
// However, we're somewhat stuck. We need to keep the references around until we restore values, but
1175+
// we don't have a use for them. The easiest solution is to simply pass this into the corresponding
1176+
// parameter for BailOutcommonNoCodeGen, but that requires us to save all of the vars, not just the
1177+
// int ones. This is ultimately significantly more predictable than attempting to manage the lifetimes
1178+
// in some other way though. We can't just do what we were doing previously, which is saving values
1179+
// here and not passing them into BailOutCommonNoCodeGen, because then the compiler will likely get
1180+
// rid of the memcpy and then the dead registerSaves array, since it can figure out that there's no
1181+
// side effect (due to the GC not being something that the optimizer can, or should, reason about).
1182+
Js::Var registerSaves[BailOutRegisterSaveSlotCount];
11751183
js_memcpy_s(registerSaves, sizeof(registerSaves), (Js::Var *)layout->functionObject->GetScriptContext()->GetThreadContext()->GetBailOutRegisterSaveSpace(),
11761184
sizeof(registerSaves));
11771185

1178-
Js::Var result = BailOutCommonNoCodeGen(layout, bailOutRecord, bailOutOffset, returnAddress, bailOutKind, branchValue, nullptr, bailOutReturnValue, argoutRestoreAddress);
1186+
Js::Var result = BailOutCommonNoCodeGen(layout, bailOutRecord, bailOutOffset, returnAddress, bailOutKind, branchValue, registerSaves, bailOutReturnValue, argoutRestoreAddress);
11791187
ScheduleFunctionCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), nullptr, bailOutRecord, bailOutKind, bailOutOffset, savedImplicitCallFlags, returnAddress);
11801188
return result;
11811189
}
@@ -1205,7 +1213,14 @@ uint32
12051213
BailOutRecord::BailOutFromLoopBodyCommon(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord, uint32 bailOutOffset,
12061214
IR::BailOutKind bailOutKind, Js::Var branchValue)
12071215
{
1208-
uint32 result = BailOutFromLoopBodyHelper(layout, bailOutRecord, bailOutOffset, bailOutKind, branchValue);
1216+
// This isn't strictly necessary if there's no allocations on this path, but because such an
1217+
// issue would be hard to notice and introduce some significant issues, we can do this copy.
1218+
// The problem from not doing this and then doing an allocation before RestoreValues is that
1219+
// the GC doesn't check the BailOutRegisterSaveSpace.
1220+
Js::Var registerSaves[BailOutRegisterSaveSlotCount];
1221+
js_memcpy_s(registerSaves, sizeof(registerSaves), (Js::Var *)layout->functionObject->GetScriptContext()->GetThreadContext()->GetBailOutRegisterSaveSpace(),
1222+
sizeof(registerSaves));
1223+
uint32 result = BailOutFromLoopBodyHelper(layout, bailOutRecord, bailOutOffset, bailOutKind, branchValue, registerSaves);
12091224
ScheduleLoopBodyCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), nullptr, bailOutRecord, bailOutKind);
12101225
return result;
12111226
}

‎lib/Backend/BailOut.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ class BailOutRecord
234234
Js::RegSlot returnValueRegSlot;
235235
};
236236
static Js::Var BailOutCommonNoCodeGen(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord,
237-
uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::Var branchValue = nullptr, Js::Var * registerSaves = nullptr,
237+
uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::Var branchValue, Js::Var * registerSaves,
238238
BailOutReturnValue * returnValue = nullptr, void * argoutRestoreAddress = nullptr);
239239
static Js::Var BailOutCommon(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord,
240240
uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::ImplicitCallFlags savedImplicitCallFlags, Js::Var branchValue = nullptr, BailOutReturnValue * returnValue = nullptr, void * argoutRestoreAddress = nullptr);
@@ -251,7 +251,7 @@ class BailOutRecord
251251
static void BailOutInlinedHelper(Js::JavascriptCallStackLayout * layout, BailOutRecord const *& bailOutRecord, uint32 bailOutOffset, void * returnAddress,
252252
IR::BailOutKind bailOutKind, Js::Var * registerSaves, BailOutReturnValue * returnValue, Js::ScriptFunction ** innerMostInlinee, bool isInLoopBody, Js::Var branchValue = nullptr);
253253
static uint32 BailOutFromLoopBodyHelper(Js::JavascriptCallStackLayout * layout, BailOutRecord const * bailOutRecord,
254-
uint32 bailOutOffset, IR::BailOutKind bailOutKind, Js::Var branchValue = nullptr, Js::Var * registerSaves = nullptr, BailOutReturnValue * returnValue = nullptr);
254+
uint32 bailOutOffset, IR::BailOutKind bailOutKind, Js::Var branchValue, Js::Var * registerSaves, BailOutReturnValue * returnValue = nullptr);
255255

256256
static void UpdatePolymorphicFieldAccess(Js::JavascriptFunction * function, BailOutRecord const * bailOutRecord);
257257

‎lib/Backend/IRBuilder.cpp‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,11 @@ IRBuilder::Build()
415415
m_func->m_headInstr->InsertAfter(m_func->m_tailInstr);
416416
m_func->m_isLeaf = true; // until proven otherwise
417417

418+
if (m_func->GetJITFunctionBody()->IsParamAndBodyScopeMerged())
419+
{
420+
this->SetParamScopeDone();
421+
}
422+
418423
if (m_func->GetJITFunctionBody()->GetLocalClosureReg() != Js::Constants::NoRegister)
419424
{
420425
m_func->InitLocalClosureSyms();
@@ -3465,9 +3470,10 @@ IRBuilder::BuildElementSlotI1(Js::OpCode newOpcode, uint32 offset, Js::RegSlot r
34653470
StackSym * stackFuncPtrSym = nullptr;
34663471
SymID symID = m_func->GetJITFunctionBody()->GetLocalClosureReg();
34673472
bool isLdSlotThatWasNotProfiled = false;
3468-
uint scopeSlotSize = m_func->GetJITFunctionBody()->GetScopeSlotArraySize();
34693473
StackSym* closureSym = m_func->GetLocalClosureSym();
34703474

3475+
uint scopeSlotSize = this->IsParamScopeDone() ? m_func->GetJITFunctionBody()->GetScopeSlotArraySize() : m_func->GetJITFunctionBody()->GetParamScopeSlotArraySize();
3476+
34713477
switch (newOpcode)
34723478
{
34733479
case Js::OpCode::LdParamSlot:
@@ -3477,7 +3483,7 @@ IRBuilder::BuildElementSlotI1(Js::OpCode newOpcode, uint32 offset, Js::RegSlot r
34773483
// Fall through
34783484

34793485
case Js::OpCode::LdLocalSlot:
3480-
if (PHASE_ON(Js::ClosureRangeCheckPhase, m_func))
3486+
if (!PHASE_OFF(Js::ClosureRangeCheckPhase, m_func))
34813487
{
34823488
if ((uint32)slotId >= scopeSlotSize + Js::ScopeSlots::FirstSlotIndex)
34833489
{
@@ -3583,7 +3589,7 @@ IRBuilder::BuildElementSlotI1(Js::OpCode newOpcode, uint32 offset, Js::RegSlot r
35833589

35843590
case Js::OpCode::StLocalSlot:
35853591
case Js::OpCode::StLocalSlotChkUndecl:
3586-
if (PHASE_ON(Js::ClosureRangeCheckPhase, m_func))
3592+
if (!PHASE_OFF(Js::ClosureRangeCheckPhase, m_func))
35873593
{
35883594
if ((uint32)slotId >= scopeSlotSize + Js::ScopeSlots::FirstSlotIndex)
35893595
{
@@ -6760,6 +6766,9 @@ IRBuilder::BuildEmpty(Js::OpCode newOpcode, uint32 offset)
67606766
// This marks the end of a param socpe which is not merged with body scope.
67616767
// So we have to first cache the closure so that we can use it to copy the initial values for
67626768
// body syms from corresponding param syms (LdParamSlot). Body should get its own scope slot.
6769+
Assert(!this->IsParamScopeDone());
6770+
this->SetParamScopeDone();
6771+
67636772
this->AddInstr(
67646773
IR::Instr::New(
67656774
Js::OpCode::Ld_A,

‎lib/Backend/IRBuilder.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ class IRBuilder
7171
, m_switchBuilder(&m_switchAdapter)
7272
, m_stackFuncPtrSym(nullptr)
7373
, m_loopBodyForInEnumeratorArrayOpnd(nullptr)
74+
, m_paramScopeDone(false)
7475
#if DBG
7576
, m_callsOnStack(0)
7677
, m_usedAsTemp(nullptr)
@@ -276,6 +277,9 @@ class IRBuilder
276277
return reg > 0 && reg < m_func->GetJITFunctionBody()->GetConstCount();
277278
}
278279

280+
bool IsParamScopeDone() const { return m_paramScopeDone; }
281+
void SetParamScopeDone(bool done = true) { m_paramScopeDone = done; }
282+
279283
Js::RegSlot InnerScopeIndexToRegSlot(uint32) const;
280284
Js::RegSlot GetEnvReg() const;
281285
Js::RegSlot GetEnvRegForEvalCode() const;
@@ -347,6 +351,7 @@ class IRBuilder
347351
StackSym * m_loopBodyRetIPSym;
348352
StackSym* m_loopCounterSym;
349353
StackSym * m_stackFuncPtrSym;
354+
bool m_paramScopeDone;
350355
bool callTreeHasSomeProfileInfo;
351356
uint finallyBlockLevel;
352357

‎lib/Backend/Lower.cpp‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8858,14 +8858,37 @@ Lowerer::LowerLdArrViewElem(IR::Instr * instr)
88588858
IR::Instr * instrPrev = instr->m_prev;
88598859

88608860
IR::RegOpnd * indexOpnd = instr->GetSrc1()->AsIndirOpnd()->GetIndexOpnd();
8861+
int32 offset = instr->GetSrc1()->AsIndirOpnd()->GetOffset();
88618862

88628863
IR::Opnd * dst = instr->GetDst();
88638864
IR::Opnd * src1 = instr->GetSrc1();
88648865
IR::Opnd * src2 = instr->GetSrc2();
88658866

88668867
IR::Instr * done;
88678868

8868-
if (indexOpnd || m_func->GetJITFunctionBody()->GetAsmJsInfo()->AccessNeedsBoundCheck((uint32)src1->AsIndirOpnd()->GetOffset()))
8869+
if (offset < 0)
8870+
{
8871+
IR::Opnd * oobValue = nullptr;
8872+
if(dst->IsFloat32())
8873+
{
8874+
oobValue = IR::MemRefOpnd::New(m_func->GetThreadContextInfo()->GetFloatNaNAddr(), TyFloat32, m_func);
8875+
}
8876+
else if(dst->IsFloat64())
8877+
{
8878+
oobValue = IR::MemRefOpnd::New(m_func->GetThreadContextInfo()->GetDoubleNaNAddr(), TyFloat64, m_func);
8879+
}
8880+
else
8881+
{
8882+
oobValue = IR::IntConstOpnd::New(0, dst->GetType(), m_func);
8883+
}
8884+
instr->ReplaceSrc1(oobValue);
8885+
if (src2)
8886+
{
8887+
instr->FreeSrc2();
8888+
}
8889+
return m_lowererMD.ChangeToAssign(instr);
8890+
}
8891+
if (indexOpnd || m_func->GetJITFunctionBody()->GetAsmJsInfo()->AccessNeedsBoundCheck((uint32)offset))
88698892
{
88708893
// CMP indexOpnd, src2(arrSize)
88718894
// JA $helper
@@ -9143,6 +9166,7 @@ Lowerer::LowerStArrViewElem(IR::Instr * instr)
91439166

91449167
// type of dst is the type of array
91459168
IR::RegOpnd * indexOpnd = dst->AsIndirOpnd()->GetIndexOpnd();
9169+
int32 offset = dst->AsIndirOpnd()->GetOffset();
91469170

91479171
Assert(!dst->IsFloat32() || src1->IsFloat32());
91489172
Assert(!dst->IsFloat64() || src1->IsFloat64());
@@ -9154,7 +9178,12 @@ Lowerer::LowerStArrViewElem(IR::Instr * instr)
91549178
{
91559179
done = LowerWasmMemOp(instr, dst);
91569180
}
9157-
else if (indexOpnd || m_func->GetJITFunctionBody()->GetAsmJsInfo()->AccessNeedsBoundCheck((uint32)dst->AsIndirOpnd()->GetOffset()))
9181+
else if (offset < 0)
9182+
{
9183+
instr->Remove();
9184+
return instrPrev;
9185+
}
9186+
else if (indexOpnd || m_func->GetJITFunctionBody()->GetAsmJsInfo()->AccessNeedsBoundCheck((uint32)offset))
91589187
{
91599188
// CMP indexOpnd, src2(arrSize)
91609189
// JA $helper

‎lib/Backend/amd64/PeepsMD.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,20 @@ PeepsMD::ProcessImplicitRegs(IR::Instr *instr)
4141
this->peeps->ClearReg(RegRDX);
4242
}
4343
}
44+
else if (instr->m_opcode == Js::OpCode::XCHG)
45+
{
46+
// At time of writing, I believe that src1 is always identical to dst, but clear both for robustness.
47+
48+
// Either of XCHG's operands (but not both) can be a memory address, so only clear registers.
49+
if (instr->GetSrc1()->IsRegOpnd())
50+
{
51+
this->peeps->ClearReg(instr->GetSrc1()->AsRegOpnd()->GetReg());
52+
}
53+
if (instr->GetSrc2()->IsRegOpnd())
54+
{
55+
this->peeps->ClearReg(instr->GetSrc2()->AsRegOpnd()->GetReg());
56+
}
57+
}
4458
}
4559

4660
void

‎lib/Backend/i386/PeepsMD.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,20 @@ PeepsMD::ProcessImplicitRegs(IR::Instr *instr)
4747
this->peeps->ClearReg(RegEDX);
4848
}
4949
}
50+
else if (instr->m_opcode == Js::OpCode::XCHG)
51+
{
52+
// At time of writing, I believe that src1 is always identical to dst, but clear both for robustness.
53+
54+
// Either of XCHG's operands (but not both) can be a memory address, so only clear registers.
55+
if (instr->GetSrc1()->IsRegOpnd())
56+
{
57+
this->peeps->ClearReg(instr->GetSrc1()->AsRegOpnd()->GetReg());
58+
}
59+
if (instr->GetSrc2()->IsRegOpnd())
60+
{
61+
this->peeps->ClearReg(instr->GetSrc2()->AsRegOpnd()->GetReg());
62+
}
63+
}
5064
}
5165

5266
void

‎lib/Parser/Hash.cpp‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -131,18 +131,6 @@ void HashTbl::Grow()
131131
#endif
132132
}
133133

134-
void HashTbl::ClearPidRefStacks()
135-
{
136-
// Clear pidrefstack pointers from all existing pid's.
137-
for (uint i = 0; i < m_luMask; i++)
138-
{
139-
for (IdentPtr pid = m_prgpidName[i]; pid; pid = pid->m_pidNext)
140-
{
141-
pid->m_pidRefStack = nullptr;
142-
}
143-
}
144-
}
145-
146134
#if DEBUG
147135
uint HashTbl::CountAndVerifyItems(IdentPtr *buckets, uint bucketCount, uint mask)
148136
{

‎lib/Parser/Hash.h‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,18 @@ class HashTbl
379379
NoReleaseAllocator* GetAllocator() {return &m_noReleaseAllocator;}
380380

381381
bool Contains(_In_reads_(cch) LPCOLESTR prgch, int32 cch);
382-
void ClearPidRefStacks();
382+
383+
template<typename Fn>
384+
void VisitPids(Fn fn)
385+
{
386+
for (uint i = 0; i <= m_luMask; i++)
387+
{
388+
for (IdentPtr pid = m_prgpidName[i]; pid; pid = pid->m_pidNext)
389+
{
390+
fn(pid);
391+
}
392+
}
393+
}
383394

384395
private:
385396
NoReleaseAllocator m_noReleaseAllocator; // to allocate identifiers

0 commit comments

Comments
 (0)