Skip to content

Commit f979536

Browse files
committed
[CVE-2017-8610] Prevent loading bad function object for inlinees
1 parent e40a34f commit f979536

File tree

6 files changed

+63
-22
lines changed

6 files changed

+63
-22
lines changed

lib/Backend/BackwardPass.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6854,10 +6854,7 @@ BackwardPass::TrackNoImplicitCallInlinees(IR::Instr *instr)
68546854
|| OpCodeAttr::CallInstr(instr->m_opcode)
68556855
|| instr->CallsAccessor()
68566856
|| GlobOpt::MayNeedBailOnImplicitCall(instr, nullptr, nullptr)
6857-
|| instr->m_opcode == Js::OpCode::LdHeapArguments
6858-
|| instr->m_opcode == Js::OpCode::LdLetHeapArguments
6859-
|| instr->m_opcode == Js::OpCode::LdHeapArgsCached
6860-
|| instr->m_opcode == Js::OpCode::LdLetHeapArgsCached
6857+
|| instr->HasAnyLoadHeapArgsOpCode()
68616858
|| instr->m_opcode == Js::OpCode::LdFuncExpr)
68626859
{
68636860
// This func has instrs with bailouts or implicit calls

lib/Backend/Func.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
9595
hasThrow(false),
9696
hasNonSimpleParams(false),
9797
hasUnoptimizedArgumentsAcccess(false),
98-
hasApplyTargetInlining(false),
98+
applyTargetInliningRemovedArgumentsAccess(false),
9999
hasImplicitCalls(false),
100100
hasTempObjectProducingInstr(false),
101101
isInlinedConstructor(isInlinedConstructor),

lib/Backend/Func.h

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -509,12 +509,24 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
509509
IR::SymOpnd *GetNextInlineeFrameArgCountSlotOpnd()
510510
{
511511
Assert(!this->m_hasInlineArgsOpt);
512+
if (this->m_hasInlineArgsOpt)
513+
{
514+
// If the function has inlineArgsOpt turned on, jitted code will not write to stack slots for inlinee's function object
515+
// and arguments, until needed. If we attempt to read from those slots, we may be reading uninitialized memory.
516+
throw Js::OperationAbortedException();
517+
}
512518
return GetInlineeOpndAtOffset((Js::Constants::InlineeMetaArgCount + actualCount) * MachPtr);
513519
}
514520

515521
IR::SymOpnd *GetInlineeFunctionObjectSlotOpnd()
516522
{
517523
Assert(!this->m_hasInlineArgsOpt);
524+
if (this->m_hasInlineArgsOpt)
525+
{
526+
// If the function has inlineArgsOpt turned on, jitted code will not write to stack slots for inlinee's function object
527+
// and arguments, until needed. If we attempt to read from those slots, we may be reading uninitialized memory.
528+
throw Js::OperationAbortedException();
529+
}
518530
return GetInlineeOpndAtOffset(Js::Constants::InlineeMetaArgIndex_FunctionObject * MachPtr);
519531
}
520532

@@ -526,6 +538,12 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
526538
IR::SymOpnd *GetInlineeArgvSlotOpnd()
527539
{
528540
Assert(!this->m_hasInlineArgsOpt);
541+
if (this->m_hasInlineArgsOpt)
542+
{
543+
// If the function has inlineArgsOpt turned on, jitted code will not write to stack slots for inlinee's function object
544+
// and arguments, until needed. If we attempt to read from those slots, we may be reading uninitialized memory.
545+
throw Js::OperationAbortedException();
546+
}
529547
return GetInlineeOpndAtOffset(Js::Constants::InlineeMetaArgIndex_Argv * MachPtr);
530548
}
531549

@@ -686,7 +704,7 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
686704
bool hasThrow : 1;
687705
bool hasUnoptimizedArgumentsAcccess : 1; // True if there are any arguments access beyond the simple case of this.apply pattern
688706
bool m_canDoInlineArgsOpt : 1;
689-
bool hasApplyTargetInlining:1;
707+
bool applyTargetInliningRemovedArgumentsAccess :1;
690708
bool isGetterSetter : 1;
691709
const bool isInlinedConstructor: 1;
692710
bool hasImplicitCalls: 1;
@@ -804,8 +822,8 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
804822
}
805823
}
806824

807-
bool GetHasApplyTargetInlining() const { return this->hasApplyTargetInlining;}
808-
void SetHasApplyTargetInlining() { this->hasApplyTargetInlining = true;}
825+
bool GetApplyTargetInliningRemovedArgumentsAccess() const { return this->applyTargetInliningRemovedArgumentsAccess;}
826+
void SetApplyTargetInliningRemovedArgumentsAccess() { this->applyTargetInliningRemovedArgumentsAccess = true;}
809827

810828
bool GetHasMarkTempObjects() const { return this->hasMarkTempObjects; }
811829
void SetHasMarkTempObjects() { this->hasMarkTempObjects = true; }

lib/Backend/Inline.cpp

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2701,6 +2701,13 @@ bool Inline::InlineApplyScriptTarget(IR::Instr *callInstr, const FunctionJITTime
27012701
return false;
27022702
});
27032703

2704+
// If the arguments object was passed in as the first argument to apply,
2705+
// 'arguments' access continues to exist even after apply target inlining
2706+
if (!HasArgumentsAccess(explicitThisArgOut))
2707+
{
2708+
callInstr->m_func->SetApplyTargetInliningRemovedArgumentsAccess();
2709+
}
2710+
27042711
if (safeThis)
27052712
{
27062713
IR::Instr * byteCodeArgOutCapture = explicitThisArgOut->GetBytecodeArgOutCapture();
@@ -3134,11 +3141,6 @@ Inline::TryGetFixedMethodsForBuiltInAndTarget(IR::Instr *callInstr, const Functi
31343141
return false;
31353142
}
31363143

3137-
if (isApplyTarget)
3138-
{
3139-
callInstr->m_func->SetHasApplyTargetInlining();
3140-
}
3141-
31423144
Assert(callInstr->m_opcode == originalCallOpCode);
31433145
callInstr->ReplaceSrc1(builtInLdInstr->GetDst());
31443146

@@ -5281,21 +5283,39 @@ Inline::IsArgumentsOpnd(IR::Opnd* opnd, SymID argumentsSymId)
52815283
return false;
52825284
}
52835285

5286+
bool
5287+
Inline::IsArgumentsOpnd(IR::Opnd* opnd)
5288+
{
5289+
IR::Opnd * checkOpnd = opnd;
5290+
while (checkOpnd)
5291+
{
5292+
if (checkOpnd->IsArgumentsObject())
5293+
{
5294+
return true;
5295+
}
5296+
checkOpnd = checkOpnd->GetStackSym() && checkOpnd->GetStackSym()->IsSingleDef() ? checkOpnd->GetStackSym()->GetInstrDef()->GetSrc1() : nullptr;
5297+
}
5298+
5299+
return false;
5300+
}
52845301

52855302
bool
52865303
Inline::HasArgumentsAccess(IR::Opnd *opnd, SymID argumentsSymId)
52875304
{
52885305
// We should look at dst last to correctly handle cases where it's the same as one of the src operands.
5289-
if (opnd)
5306+
IR::Opnd * checkOpnd = opnd;
5307+
while (checkOpnd)
52905308
{
5291-
if (opnd->IsRegOpnd() || opnd->IsSymOpnd() || opnd->IsIndirOpnd())
5309+
if (checkOpnd->IsRegOpnd() || checkOpnd->IsSymOpnd() || checkOpnd->IsIndirOpnd())
52925310
{
5293-
if (IsArgumentsOpnd(opnd, argumentsSymId))
5311+
if (IsArgumentsOpnd(checkOpnd, argumentsSymId))
52945312
{
52955313
return true;
52965314
}
52975315
}
5316+
checkOpnd = checkOpnd->GetStackSym() && checkOpnd->GetStackSym()->IsSingleDef() ? checkOpnd->GetStackSym()->GetInstrDef()->GetSrc1() : nullptr;
52985317
}
5318+
52995319
return false;
53005320
}
53015321

@@ -5328,6 +5348,13 @@ Inline::HasArgumentsAccess(IR::Instr * instr, SymID argumentsSymId)
53285348
return false;
53295349
}
53305350

5351+
bool
5352+
Inline::HasArgumentsAccess(IR::Instr * instr)
5353+
{
5354+
return (instr->GetSrc1() && IsArgumentsOpnd(instr->GetSrc1())) ||
5355+
(instr->GetSrc2() && IsArgumentsOpnd(instr->GetSrc2()));
5356+
}
5357+
53315358
bool
53325359
Inline::GetInlineeHasArgumentObject(Func * inlinee)
53335360
{
@@ -5339,14 +5366,12 @@ Inline::GetInlineeHasArgumentObject(Func * inlinee)
53395366

53405367
// Inlinee has arguments access
53415368

5342-
if (!inlinee->GetHasApplyTargetInlining())
5369+
if (!inlinee->GetApplyTargetInliningRemovedArgumentsAccess())
53435370
{
5344-
// There is no apply target inlining (this.init.apply(this, arguments))
5345-
// So arguments access continues to exist
53465371
return true;
53475372
}
53485373

5349-
// Its possible there is no more arguments access after we inline apply target validate the same.
5374+
// Its possible there is no more arguments access after we inline apply target; validate the same.
53505375
// This sounds expensive, but we are only walking inlinee which has apply target inlining optimization enabled.
53515376
// Also we walk only instruction in that inlinee and not nested inlinees. So it is not expensive.
53525377
SymID argumentsSymId = 0;

lib/Backend/Inline.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,10 @@ class Inline
9191
IR::Instr * RemoveLdThis(IR::Instr *instr);
9292
bool GetInlineeHasArgumentObject(Func * inlinee);
9393
bool HasArgumentsAccess(IR::Instr * instr, SymID argumentsSymId);
94+
bool HasArgumentsAccess(IR::Instr * instr);
9495
bool HasArgumentsAccess(IR::Opnd * opnd, SymID argumentsSymId);
9596
bool IsArgumentsOpnd(IR::Opnd* opnd,SymID argumentsSymId);
97+
bool IsArgumentsOpnd(IR::Opnd* opnd);
9698
void Cleanup(IR::Instr *callInstr);
9799
IR::PropertySymOpnd* GetMethodLdOpndForCallInstr(IR::Instr* callInstr);
98100
#ifdef ENABLE_SIMDJS

lib/Backend/Opnd.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2799,8 +2799,7 @@ Opnd::IsArgumentsObject()
27992799
// Since we need this information in the inliner where we don't track arguments object sym, going with single def is the best option.
28002800
StackSym * sym = this->GetStackSym();
28012801

2802-
return sym && sym->IsSingleDef() &&
2803-
(sym->m_instrDef->m_opcode == Js::OpCode::LdHeapArguments || sym->m_instrDef->m_opcode == Js::OpCode::LdLetHeapArguments);
2802+
return sym && sym->IsSingleDef() && sym->GetInstrDef()->HasAnyLoadHeapArgsOpCode();
28042803
}
28052804

28062805
#if DBG_DUMP || defined(ENABLE_IR_VIEWER)

0 commit comments

Comments
 (0)