Skip to content

Commit 2637140

Browse files
pleathrajatd
authored andcommitted
[CVE-2018-0979] Incorrect byte code can cause dereference of uninitialized stack location - Internal
1 parent 4bdd0ef commit 2637140

File tree

9 files changed

+121
-107
lines changed

9 files changed

+121
-107
lines changed

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Lines changed: 70 additions & 70 deletions
Large diffs are not rendered by default.

lib/Runtime/ByteCode/ByteCodeGenerator.cpp

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2545,7 +2545,7 @@ FuncInfo* PreVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerato
25452545
}
25462546
PreVisitBlock(pnode->sxFnc.pnodeScopes, byteCodeGenerator);
25472547
// If we have arguments, we are going to need locations if the function is in strict mode or we have a non-simple parameter list. This is because we will not create a scope object.
2548-
bool assignLocationForFormals = !ByteCodeGenerator::NeedScopeObjectForArguments(funcInfo, funcInfo->root);
2548+
bool assignLocationForFormals = !byteCodeGenerator->NeedScopeObjectForArguments(funcInfo, funcInfo->root);
25492549
AddArgsToScope(pnode, byteCodeGenerator, assignLocationForFormals);
25502550

25512551
return funcInfo;
@@ -2605,7 +2605,7 @@ void AssignFuncSymRegister(ParseNode * pnode, ByteCodeGenerator * byteCodeGenera
26052605
Symbol * functionScopeVarSym = sym->GetFuncScopeVarSym();
26062606
if (functionScopeVarSym &&
26072607
!functionScopeVarSym->GetIsGlobal() &&
2608-
!functionScopeVarSym->IsInSlot(sym->GetScope()->GetFunc()))
2608+
!functionScopeVarSym->IsInSlot(byteCodeGenerator, sym->GetScope()->GetFunc()))
26092609
{
26102610
byteCodeGenerator->AssignRegister(functionScopeVarSym);
26112611
}
@@ -2780,7 +2780,7 @@ FuncInfo* PostVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerat
27802780
{
27812781
if (top->GetCallsEval() ||
27822782
top->GetChildCallsEval() ||
2783-
(top->GetHasArguments() && ByteCodeGenerator::NeedScopeObjectForArguments(top, pnode)) ||
2783+
(top->GetHasArguments() && byteCodeGenerator->NeedScopeObjectForArguments(top, pnode)) ||
27842784
top->GetHasLocalInClosure() ||
27852785
(top->funcExprScope && top->funcExprScope->GetMustInstantiate()) ||
27862786
// When we have split scope normally either eval will be present or the GetHasLocalInClosure will be true as one of the formal is
@@ -2849,10 +2849,10 @@ FuncInfo* PostVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerat
28492849
};
28502850

28512851
// We need to include the rest as well -as it will get slot assigned.
2852-
if (ByteCodeGenerator::NeedScopeObjectForArguments(top, pnode))
2852+
if (byteCodeGenerator->NeedScopeObjectForArguments(top, pnode))
28532853
{
28542854
MapFormals(pnode, setArgScopeSlot);
2855-
if (argSym->NeedsSlotAlloc(top))
2855+
if (argSym->NeedsSlotAlloc(byteCodeGenerator, top))
28562856
{
28572857
Assert(argSym->GetScopeSlot() == Js::Constants::NoProperty);
28582858
argSym->SetScopeSlot(i++);
@@ -2863,7 +2863,7 @@ FuncInfo* PostVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerat
28632863
top->paramScope->SetScopeSlotCount(i);
28642864

28652865
Assert(top->GetHasHeapArguments());
2866-
if (ByteCodeGenerator::NeedScopeObjectForArguments(top, pnode)
2866+
if (byteCodeGenerator->NeedScopeObjectForArguments(top, pnode)
28672867
&& !pnode->sxFnc.HasNonSimpleParameterList())
28682868
{
28692869
top->byteCodeFunction->SetHasImplicitArgIns(false);
@@ -3068,7 +3068,7 @@ void ByteCodeGenerator::ProcessCapturedSym(Symbol *sym)
30683068
FuncInfo *funcHome = sym->GetScope()->GetFunc();
30693069
FuncInfo *funcChild = funcHome->GetCurrentChildFunction();
30703070

3071-
Assert(sym->NeedsSlotAlloc(funcHome) || sym->GetIsGlobal() || sym->GetIsModuleImport() || sym->GetIsModuleExportStorage());
3071+
Assert(sym->NeedsSlotAlloc(this, funcHome) || sym->GetIsGlobal() || sym->GetIsModuleImport() || sym->GetIsModuleExportStorage());
30723072

30733073
// If this is not a local property, or not all its references can be tracked, or
30743074
// it's not scoped to the function, or we're in debug mode, disable the delayed capture optimization.
@@ -4953,11 +4953,11 @@ void AssignRegisters(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator)
49534953
}
49544954
// Don't give the declared var a register if it's in a closure, because the closure slot
49554955
// is its true "home". (Need to check IsGlobal again as the sym may have changed above.)
4956-
if (!sym->GetIsGlobal() && !sym->IsInSlot(funcInfo))
4956+
if (!sym->GetIsGlobal() && !sym->IsInSlot(byteCodeGenerator, funcInfo))
49574957
{
49584958
if (PHASE_TRACE(Js::DelayCapturePhase, funcInfo->byteCodeFunction))
49594959
{
4960-
if (sym->NeedsSlotAlloc(byteCodeGenerator->TopFuncInfo()))
4960+
if (sym->NeedsSlotAlloc(byteCodeGenerator, byteCodeGenerator->TopFuncInfo()))
49614961
{
49624962
Output::Print(_u("--- DelayCapture: Delayed capturing symbol '%s' during initialization.\n"),
49634963
sym->GetName().GetBuffer());
@@ -5011,12 +5011,12 @@ void AssignRegisters(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator)
50115011
if (!sym->GetIsGlobal() &&
50125012
!sym->GetIsMember() &&
50135013
byteCodeGenerator->TopFuncInfo() == sym->GetScope()->GetEnclosingFunc() &&
5014-
!sym->IsInSlot(byteCodeGenerator->TopFuncInfo()) &&
5014+
!sym->IsInSlot(byteCodeGenerator, byteCodeGenerator->TopFuncInfo()) &&
50155015
!sym->HasVisitedCapturingFunc())
50165016
{
50175017
if (PHASE_TRACE(Js::DelayCapturePhase, byteCodeGenerator->TopFuncInfo()->byteCodeFunction))
50185018
{
5019-
if (sym->NeedsSlotAlloc(byteCodeGenerator->TopFuncInfo()))
5019+
if (sym->NeedsSlotAlloc(byteCodeGenerator, byteCodeGenerator->TopFuncInfo()))
50205020
{
50215021
Output::Print(_u("--- DelayCapture: Delayed capturing symbol '%s'.\n"),
50225022
sym->GetName().GetBuffer());
@@ -5177,8 +5177,7 @@ Js::FunctionBody * ByteCodeGenerator::MakeGlobalFunctionBody(ParseNode *pnode)
51775177
return func;
51785178
}
51795179

5180-
/* static */
5181-
bool ByteCodeGenerator::NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNode *pnodeFnc)
5180+
bool ByteCodeGenerator::NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNode *pnodeFnc) const
51825181
{
51835182
// We can avoid creating a scope object with arguments present if:
51845183
bool dontNeedScopeObject =
@@ -5187,6 +5186,8 @@ bool ByteCodeGenerator::NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNod
51875186
// Either we are in strict mode, or have strict mode formal semantics from a non-simple parameter list, and
51885187
&& (funcInfo->GetIsStrictMode()
51895188
|| pnodeFnc->sxFnc.HasNonSimpleParameterList())
5189+
// We're not in eval or event handler, which will force the scope(s) to be objects
5190+
&& !(this->flags & (fscrEval | fscrImplicitThis | fscrImplicitParents))
51905191
// Neither of the scopes are objects
51915192
&& !funcInfo->paramScope->GetIsObject()
51925193
&& !funcInfo->bodyScope->GetIsObject();

lib/Runtime/ByteCode/ByteCodeGenerator.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,10 +398,12 @@ class ByteCodeGenerator
398398
Js::FunctionBody *EnsureFakeGlobalFuncForUndefer(ParseNode *pnode);
399399
Js::FunctionBody *MakeGlobalFunctionBody(ParseNode *pnode);
400400

401-
static bool NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNode *pnodeFnc);
401+
bool NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNode *pnodeFnc) const;
402402

403403
void AddFuncInfoToFinalizationSet(FuncInfo *funcInfo);
404404
void FinalizeFuncInfos();
405+
void CheckFncDeclScopeSlot(ParseNode *pnodeFnc, FuncInfo *funcInfo);
406+
void EnsureFncDeclScopeSlot(ParseNode *pnodeFnc, FuncInfo *funcInfo);
405407

406408
Js::OpCode GetStSlotOp(Scope *scope, int envIndex, Js::RegSlot scopeLocation, bool chkBlockVar, FuncInfo *funcInfo);
407409
Js::OpCode GetLdSlotOp(Scope *scope, int envIndex, Js::RegSlot scopeLocation, FuncInfo *funcInfo);

lib/Runtime/ByteCode/ScopeInfo.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace Js
2222
else if (needScopeSlot)
2323
{
2424
// Any symbol may have non-local ref from deferred child. Allocate slot for it.
25-
scopeSlot = sym->EnsureScopeSlot(mapSymbolData->func);
25+
scopeSlot = sym->EnsureScopeSlot(mapSymbolData->byteCodeGenerator, mapSymbolData->func);
2626
}
2727

2828
if (needScopeSlot || sym->GetIsModuleExportStorage())
@@ -52,7 +52,7 @@ namespace Js
5252
//
5353
// Create scope info for a single scope.
5454
//
55-
ScopeInfo* ScopeInfo::SaveOneScopeInfo(/*ByteCodeGenerator* byteCodeGenerator, ParseableFunctionInfo* parent,*/ Scope* scope, ScriptContext *scriptContext)
55+
ScopeInfo* ScopeInfo::SaveOneScopeInfo(ByteCodeGenerator* byteCodeGenerator, Scope* scope, ScriptContext *scriptContext)
5656
{
5757
Assert(scope->GetScopeInfo() == nullptr);
5858
Assert(scope->GetScopeType() != ScopeType_Global);
@@ -82,7 +82,7 @@ namespace Js
8282
scope->GetFunc()->name, count,
8383
scopeInfo->isObject ? _u("isObject") : _u(""));
8484

85-
MapSymbolData mapSymbolData = { scope->GetFunc(), 0 };
85+
MapSymbolData mapSymbolData = { byteCodeGenerator, scope->GetFunc(), 0 };
8686
scope->ForEachSymbol([&mapSymbolData, scopeInfo, scope](Symbol * sym)
8787
{
8888
Assert(scope == sym->GetScope());
@@ -109,7 +109,7 @@ namespace Js
109109
//
110110
// Save scope info for an individual scope and link it to its enclosing scope.
111111
//
112-
ScopeInfo * ScopeInfo::SaveScopeInfo(Scope * scope/*ByteCodeGenerator* byteCodeGenerator, FuncInfo* parentFunc, FuncInfo* func*/, ScriptContext * scriptContext)
112+
ScopeInfo * ScopeInfo::SaveScopeInfo(ByteCodeGenerator* byteCodeGenerator, Scope * scope, ScriptContext * scriptContext)
113113
{
114114
// Advance past scopes that will be excluded from the closure environment. (But note that we always want the body scope.)
115115
while (scope && (!scope->GetMustInstantiate() && scope != scope->GetFunc()->GetBodyScope()))
@@ -131,13 +131,13 @@ namespace Js
131131
}
132132

133133
// Do the work for this scope.
134-
scopeInfo = ScopeInfo::SaveOneScopeInfo(scope, scriptContext);
134+
scopeInfo = ScopeInfo::SaveOneScopeInfo(byteCodeGenerator, scope, scriptContext);
135135

136136
// Link to the parent (if any).
137137
scope = scope->GetEnclosingScope();
138138
if (scope)
139139
{
140-
scopeInfo->SetParentScopeInfo(ScopeInfo::SaveScopeInfo(scope, scriptContext));
140+
scopeInfo->SetParentScopeInfo(ScopeInfo::SaveScopeInfo(byteCodeGenerator, scope, scriptContext));
141141
}
142142

143143
return scopeInfo;
@@ -167,7 +167,7 @@ namespace Js
167167
currentScope = currentScope->GetEnclosingScope();
168168
}
169169

170-
ScopeInfo * scopeInfo = ScopeInfo::SaveScopeInfo(currentScope, byteCodeGenerator->GetScriptContext());
170+
ScopeInfo * scopeInfo = ScopeInfo::SaveScopeInfo(byteCodeGenerator, currentScope, byteCodeGenerator->GetScriptContext());
171171
if (scopeInfo != nullptr)
172172
{
173173
funcInfo->byteCodeFunction->SetScopeInfo(scopeInfo);

lib/Runtime/ByteCode/ScopeInfo.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ namespace Js {
1515

1616
struct MapSymbolData
1717
{
18+
ByteCodeGenerator *byteCodeGenerator;
1819
FuncInfo* func;
1920
int nonScopeSymbolCount;
2021
};
@@ -181,8 +182,8 @@ namespace Js {
181182

182183
void SaveSymbolInfo(Symbol* sym, MapSymbolData* mapSymbolData);
183184

184-
static ScopeInfo* SaveScopeInfo(Scope * scope, ScriptContext * scriptContext);
185-
static ScopeInfo* SaveOneScopeInfo(Scope * scope, ScriptContext * scriptContext);
185+
static ScopeInfo* SaveScopeInfo(ByteCodeGenerator * byteCodeGenerator, Scope * scope, ScriptContext * scriptContext);
186+
static ScopeInfo* SaveOneScopeInfo(ByteCodeGenerator * byteCodeGenerator, Scope * scope, ScriptContext * scriptContext);
186187

187188
public:
188189
FunctionInfo * GetFunctionInfo() const

lib/Runtime/ByteCode/Symbol.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,18 @@ void Symbol::SaveToPropIdArray(Symbol *sym, Js::PropertyIdArray *propIds, ByteCo
6464
}
6565
}
6666

67-
bool Symbol::NeedsSlotAlloc(FuncInfo *funcInfo)
67+
bool Symbol::NeedsSlotAlloc(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo)
6868
{
69-
return IsInSlot(funcInfo, true);
69+
return IsInSlot(byteCodeGenerator, funcInfo, true);
7070
}
7171

72-
bool Symbol::IsInSlot(FuncInfo *funcInfo, bool ensureSlotAlloc)
72+
bool Symbol::IsInSlot(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool ensureSlotAlloc)
7373
{
7474
if (this->GetIsGlobal() || this->GetIsModuleExportStorage())
7575
{
7676
return false;
7777
}
78-
if (funcInfo->GetHasHeapArguments() && this->GetIsFormal() && ByteCodeGenerator::NeedScopeObjectForArguments(funcInfo, funcInfo->root))
78+
if (funcInfo->GetHasHeapArguments() && this->GetIsFormal() && byteCodeGenerator->NeedScopeObjectForArguments(funcInfo, funcInfo->root))
7979
{
8080
return true;
8181
}
@@ -100,9 +100,9 @@ bool Symbol::GetIsCommittedToSlot() const
100100
return isCommittedToSlot || this->scope->GetFunc()->GetCallsEval() || this->scope->GetFunc()->GetChildCallsEval();
101101
}
102102

103-
Js::PropertyId Symbol::EnsureScopeSlot(FuncInfo *funcInfo)
103+
Js::PropertyId Symbol::EnsureScopeSlot(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo)
104104
{
105-
if (this->NeedsSlotAlloc(funcInfo) && this->scopeSlot == Js::Constants::NoProperty)
105+
if (this->NeedsSlotAlloc(byteCodeGenerator, funcInfo) && this->scopeSlot == Js::Constants::NoProperty)
106106
{
107107
this->scopeSlot = this->scope->AddScopeSlot();
108108
}

lib/Runtime/ByteCode/Symbol.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,9 +480,9 @@ class Symbol
480480
return this->name;
481481
}
482482

483-
Js::PropertyId EnsureScopeSlot(FuncInfo *funcInfo);
484-
bool IsInSlot(FuncInfo *funcInfo, bool ensureSlotAlloc = false);
485-
bool NeedsSlotAlloc(FuncInfo *funcInfo);
483+
Js::PropertyId EnsureScopeSlot(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo);
484+
bool IsInSlot(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool ensureSlotAlloc = false);
485+
bool NeedsSlotAlloc(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo);
486486

487487
static void SaveToPropIdArray(Symbol *sym, Js::PropertyIdArray *propIds, ByteCodeGenerator *byteCodeGenerator, Js::PropertyId *pFirstSlot = nullptr);
488488

lib/Runtime/Language/AsmJsModule.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ namespace Js
124124
if (funcInfo->byteCodeFunction->GetIsNamedFunctionExpression())
125125
{
126126
Assert(GetModuleFunctionNode()->sxFnc.pnodeName);
127-
if (GetModuleFunctionNode()->sxFnc.pnodeName->sxVar.sym->IsInSlot(funcInfo))
127+
if (GetModuleFunctionNode()->sxFnc.pnodeName->sxVar.sym->IsInSlot(GetByteCodeGenerator(), funcInfo))
128128
{
129129
ParseNodePtr nameNode = GetModuleFunctionNode()->sxFnc.pnodeName;
130130
GetByteCodeGenerator()->AssignPropertyId(nameNode->name());

lib/Runtime/Language/InterpreterStackFrame.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,10 +1296,20 @@ namespace Js
12961296
// In the debug mode zero out the local slot, so this could prevent locals being uninitialized in the case of setNextStatement.
12971297
memset(newInstance->m_localSlots, 0, sizeof(Js::Var) * localCount);
12981298
}
1299-
// Zero out only the return slot. This is not a user local, so the byte code will not initialize
1300-
// it to "undefined". And it's not an expression temp, so, for instance, a jitted loop body may expect
1301-
// it to be valid on entry to the loop, where "valid" means either a var or null.
1302-
newInstance->SetNonVarReg(0, NULL);
1299+
else
1300+
{
1301+
Js::RegSlot varCount = function->GetFunctionBody()->GetVarCount();
1302+
if (varCount)
1303+
{
1304+
// Zero out the non-constant var slots.
1305+
Js::RegSlot constantCount = function->GetFunctionBody()->GetConstantCount();
1306+
memset(newInstance->m_localSlots + constantCount, 0, varCount * sizeof(Js::Var));
1307+
}
1308+
// Zero out the return slot. This is not a user local, so the byte code will not initialize
1309+
// it to "undefined". And it's not an expression temp, so, for instance, a jitted loop body may expect
1310+
// it to be valid on entry to the loop, where "valid" means either a var or null.
1311+
newInstance->SetNonVarReg(0, NULL);
1312+
}
13031313
#endif
13041314
// Wasm doesn't use const table
13051315
if (!executeFunction->IsWasmFunction())

0 commit comments

Comments
 (0)