[CVE-2018-0979] Incorrect byte code can cause dereference of uninitia…

…lized stack location - Internal

lib/Runtime/ByteCode/ByteCodeGenerator.cpp

@@ -2545,7 +2545,7 @@ FuncInfo* PreVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerato
     }
     PreVisitBlock(pnode->sxFnc.pnodeScopes, byteCodeGenerator);
     // 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.
-    bool assignLocationForFormals = !ByteCodeGenerator::NeedScopeObjectForArguments(funcInfo, funcInfo->root);
+    bool assignLocationForFormals = !byteCodeGenerator->NeedScopeObjectForArguments(funcInfo, funcInfo->root);
     AddArgsToScope(pnode, byteCodeGenerator, assignLocationForFormals);
 
     return funcInfo;
@@ -2605,7 +2605,7 @@ void AssignFuncSymRegister(ParseNode * pnode, ByteCodeGenerator * byteCodeGenera
                 Symbol * functionScopeVarSym = sym->GetFuncScopeVarSym();
                 if (functionScopeVarSym &&
                     !functionScopeVarSym->GetIsGlobal() &&
-                    !functionScopeVarSym->IsInSlot(sym->GetScope()->GetFunc()))
+                    !functionScopeVarSym->IsInSlot(byteCodeGenerator, sym->GetScope()->GetFunc()))
                 {
                     byteCodeGenerator->AssignRegister(functionScopeVarSym);
                 }
@@ -2780,7 +2780,7 @@ FuncInfo* PostVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerat
         {
             if (top->GetCallsEval() ||
                 top->GetChildCallsEval() ||
-                (top->GetHasArguments() && ByteCodeGenerator::NeedScopeObjectForArguments(top, pnode)) ||
+                (top->GetHasArguments() && byteCodeGenerator->NeedScopeObjectForArguments(top, pnode)) ||
                 top->GetHasLocalInClosure() ||
                 (top->funcExprScope && top->funcExprScope->GetMustInstantiate()) ||
                 // 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
                     };
 
                     // We need to include the rest as well -as it will get slot assigned.
-                    if (ByteCodeGenerator::NeedScopeObjectForArguments(top, pnode))
+                    if (byteCodeGenerator->NeedScopeObjectForArguments(top, pnode))
                     {
                         MapFormals(pnode, setArgScopeSlot);
-                        if (argSym->NeedsSlotAlloc(top))
+                        if (argSym->NeedsSlotAlloc(byteCodeGenerator, top))
                         {
                             Assert(argSym->GetScopeSlot() == Js::Constants::NoProperty);
                             argSym->SetScopeSlot(i++);
@@ -2863,7 +2863,7 @@ FuncInfo* PostVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerat
                     top->paramScope->SetScopeSlotCount(i);
 
                     Assert(top->GetHasHeapArguments());
-                    if (ByteCodeGenerator::NeedScopeObjectForArguments(top, pnode)
+                    if (byteCodeGenerator->NeedScopeObjectForArguments(top, pnode)
                         && !pnode->sxFnc.HasNonSimpleParameterList())
                     {
                         top->byteCodeFunction->SetHasImplicitArgIns(false);
@@ -3068,7 +3068,7 @@ void ByteCodeGenerator::ProcessCapturedSym(Symbol *sym)
     FuncInfo *funcHome = sym->GetScope()->GetFunc();
     FuncInfo *funcChild = funcHome->GetCurrentChildFunction();
 
-    Assert(sym->NeedsSlotAlloc(funcHome) || sym->GetIsGlobal() || sym->GetIsModuleImport() || sym->GetIsModuleExportStorage());
+    Assert(sym->NeedsSlotAlloc(this, funcHome) || sym->GetIsGlobal() || sym->GetIsModuleImport() || sym->GetIsModuleExportStorage());
 
     // If this is not a local property, or not all its references can be tracked, or
     // 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)
                 }
                 // Don't give the declared var a register if it's in a closure, because the closure slot
                 // is its true "home". (Need to check IsGlobal again as the sym may have changed above.)
-                if (!sym->GetIsGlobal() && !sym->IsInSlot(funcInfo))
+                if (!sym->GetIsGlobal() && !sym->IsInSlot(byteCodeGenerator, funcInfo))
                 {
                     if (PHASE_TRACE(Js::DelayCapturePhase, funcInfo->byteCodeFunction))
                     {
-                        if (sym->NeedsSlotAlloc(byteCodeGenerator->TopFuncInfo()))
+                        if (sym->NeedsSlotAlloc(byteCodeGenerator, byteCodeGenerator->TopFuncInfo()))
                         {
                             Output::Print(_u("--- DelayCapture: Delayed capturing symbol '%s' during initialization.\n"),
                                 sym->GetName().GetBuffer());
@@ -5011,12 +5011,12 @@ void AssignRegisters(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator)
             if (!sym->GetIsGlobal() &&
                 !sym->GetIsMember() &&
                 byteCodeGenerator->TopFuncInfo() == sym->GetScope()->GetEnclosingFunc() &&
-                !sym->IsInSlot(byteCodeGenerator->TopFuncInfo()) &&
+                !sym->IsInSlot(byteCodeGenerator, byteCodeGenerator->TopFuncInfo()) &&
                 !sym->HasVisitedCapturingFunc())
             {
                 if (PHASE_TRACE(Js::DelayCapturePhase, byteCodeGenerator->TopFuncInfo()->byteCodeFunction))
                 {
-                    if (sym->NeedsSlotAlloc(byteCodeGenerator->TopFuncInfo()))
+                    if (sym->NeedsSlotAlloc(byteCodeGenerator, byteCodeGenerator->TopFuncInfo()))
                     {
                         Output::Print(_u("--- DelayCapture: Delayed capturing symbol '%s'.\n"),
                             sym->GetName().GetBuffer());
@@ -5177,8 +5177,7 @@ Js::FunctionBody * ByteCodeGenerator::MakeGlobalFunctionBody(ParseNode *pnode)
     return func;
 }
 
-/* static */
-bool ByteCodeGenerator::NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNode *pnodeFnc)
+bool ByteCodeGenerator::NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNode *pnodeFnc) const
 {
     // We can avoid creating a scope object with arguments present if:
     bool dontNeedScopeObject =
@@ -5187,6 +5186,8 @@ bool ByteCodeGenerator::NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNod
         // Either we are in strict mode, or have strict mode formal semantics from a non-simple parameter list, and
         && (funcInfo->GetIsStrictMode()
             || pnodeFnc->sxFnc.HasNonSimpleParameterList())
+        // We're not in eval or event handler, which will force the scope(s) to be objects
+        && !(this->flags & (fscrEval | fscrImplicitThis | fscrImplicitParents))
         // Neither of the scopes are objects
         && !funcInfo->paramScope->GetIsObject()
         && !funcInfo->bodyScope->GetIsObject();

lib/Runtime/ByteCode/ByteCodeGenerator.h

@@ -398,10 +398,12 @@ class ByteCodeGenerator
     Js::FunctionBody *EnsureFakeGlobalFuncForUndefer(ParseNode *pnode);
     Js::FunctionBody *MakeGlobalFunctionBody(ParseNode *pnode);
 
-    static bool NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNode *pnodeFnc);
+    bool NeedScopeObjectForArguments(FuncInfo *funcInfo, ParseNode *pnodeFnc) const;
 
     void AddFuncInfoToFinalizationSet(FuncInfo *funcInfo);
     void FinalizeFuncInfos();
+    void CheckFncDeclScopeSlot(ParseNode *pnodeFnc, FuncInfo *funcInfo);
+    void EnsureFncDeclScopeSlot(ParseNode *pnodeFnc, FuncInfo *funcInfo);
 
     Js::OpCode GetStSlotOp(Scope *scope, int envIndex, Js::RegSlot scopeLocation, bool chkBlockVar, FuncInfo *funcInfo);
     Js::OpCode GetLdSlotOp(Scope *scope, int envIndex, Js::RegSlot scopeLocation, FuncInfo *funcInfo);

lib/Runtime/ByteCode/ScopeInfo.cpp

@@ -22,7 +22,7 @@ namespace Js
         else if (needScopeSlot)
         {
             // Any symbol may have non-local ref from deferred child. Allocate slot for it.
-            scopeSlot = sym->EnsureScopeSlot(mapSymbolData->func);
+            scopeSlot = sym->EnsureScopeSlot(mapSymbolData->byteCodeGenerator, mapSymbolData->func);
         }
 
         if (needScopeSlot || sym->GetIsModuleExportStorage())
@@ -52,7 +52,7 @@ namespace Js
     //
     // Create scope info for a single scope.
     //
-    ScopeInfo* ScopeInfo::SaveOneScopeInfo(/*ByteCodeGenerator* byteCodeGenerator, ParseableFunctionInfo* parent,*/ Scope* scope, ScriptContext *scriptContext)
+    ScopeInfo* ScopeInfo::SaveOneScopeInfo(ByteCodeGenerator* byteCodeGenerator, Scope* scope, ScriptContext *scriptContext)
     {
         Assert(scope->GetScopeInfo() == nullptr);
         Assert(scope->GetScopeType() != ScopeType_Global);
@@ -82,7 +82,7 @@ namespace Js
             scope->GetFunc()->name, count,
             scopeInfo->isObject ? _u("isObject") : _u(""));
 
-        MapSymbolData mapSymbolData = { scope->GetFunc(), 0 };
+        MapSymbolData mapSymbolData = { byteCodeGenerator, scope->GetFunc(), 0 };
         scope->ForEachSymbol([&mapSymbolData, scopeInfo, scope](Symbol * sym)
         {
             Assert(scope == sym->GetScope());
@@ -109,7 +109,7 @@ namespace Js
     //
     // Save scope info for an individual scope and link it to its enclosing scope.
     //
-    ScopeInfo * ScopeInfo::SaveScopeInfo(Scope * scope/*ByteCodeGenerator* byteCodeGenerator, FuncInfo* parentFunc, FuncInfo* func*/, ScriptContext * scriptContext)
+    ScopeInfo * ScopeInfo::SaveScopeInfo(ByteCodeGenerator* byteCodeGenerator, Scope * scope, ScriptContext * scriptContext)
     {
         // Advance past scopes that will be excluded from the closure environment. (But note that we always want the body scope.)
         while (scope && (!scope->GetMustInstantiate() && scope != scope->GetFunc()->GetBodyScope()))
@@ -131,13 +131,13 @@ namespace Js
         }
 
         // Do the work for this scope.
-        scopeInfo = ScopeInfo::SaveOneScopeInfo(scope, scriptContext);
+        scopeInfo = ScopeInfo::SaveOneScopeInfo(byteCodeGenerator, scope, scriptContext);
 
         // Link to the parent (if any).
         scope = scope->GetEnclosingScope();
         if (scope)
         {
-            scopeInfo->SetParentScopeInfo(ScopeInfo::SaveScopeInfo(scope, scriptContext));
+            scopeInfo->SetParentScopeInfo(ScopeInfo::SaveScopeInfo(byteCodeGenerator, scope, scriptContext));
         }
 
         return scopeInfo;
@@ -167,7 +167,7 @@ namespace Js
             currentScope = currentScope->GetEnclosingScope();
         }
 
-        ScopeInfo * scopeInfo = ScopeInfo::SaveScopeInfo(currentScope, byteCodeGenerator->GetScriptContext());
+        ScopeInfo * scopeInfo = ScopeInfo::SaveScopeInfo(byteCodeGenerator, currentScope, byteCodeGenerator->GetScriptContext());
         if (scopeInfo != nullptr)
         {
             funcInfo->byteCodeFunction->SetScopeInfo(scopeInfo);

lib/Runtime/ByteCode/ScopeInfo.h

@@ -15,6 +15,7 @@ namespace Js {
 
         struct MapSymbolData
         {
+            ByteCodeGenerator *byteCodeGenerator;
             FuncInfo* func;
             int nonScopeSymbolCount;
         };
@@ -181,8 +182,8 @@ namespace Js {
 
         void SaveSymbolInfo(Symbol* sym, MapSymbolData* mapSymbolData);
 
-        static ScopeInfo* SaveScopeInfo(Scope * scope, ScriptContext * scriptContext);
-        static ScopeInfo* SaveOneScopeInfo(Scope * scope, ScriptContext * scriptContext);
+        static ScopeInfo* SaveScopeInfo(ByteCodeGenerator * byteCodeGenerator, Scope * scope, ScriptContext * scriptContext);
+        static ScopeInfo* SaveOneScopeInfo(ByteCodeGenerator * byteCodeGenerator, Scope * scope, ScriptContext * scriptContext);
 
     public:
         FunctionInfo * GetFunctionInfo() const

lib/Runtime/ByteCode/Symbol.cpp

@@ -64,18 +64,18 @@ void Symbol::SaveToPropIdArray(Symbol *sym, Js::PropertyIdArray *propIds, ByteCo
     }
 }
 
-bool Symbol::NeedsSlotAlloc(FuncInfo *funcInfo)
+bool Symbol::NeedsSlotAlloc(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo)
 {
-    return IsInSlot(funcInfo, true);
+    return IsInSlot(byteCodeGenerator, funcInfo, true);
 }
 
-bool Symbol::IsInSlot(FuncInfo *funcInfo, bool ensureSlotAlloc)
+bool Symbol::IsInSlot(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool ensureSlotAlloc)
 {
     if (this->GetIsGlobal() || this->GetIsModuleExportStorage())
     {
         return false;
     }
-    if (funcInfo->GetHasHeapArguments() && this->GetIsFormal() && ByteCodeGenerator::NeedScopeObjectForArguments(funcInfo, funcInfo->root))
+    if (funcInfo->GetHasHeapArguments() && this->GetIsFormal() && byteCodeGenerator->NeedScopeObjectForArguments(funcInfo, funcInfo->root))
     {
         return true;
     }
@@ -100,9 +100,9 @@ bool Symbol::GetIsCommittedToSlot() const
     return isCommittedToSlot || this->scope->GetFunc()->GetCallsEval() || this->scope->GetFunc()->GetChildCallsEval();
 }
 
-Js::PropertyId Symbol::EnsureScopeSlot(FuncInfo *funcInfo)
+Js::PropertyId Symbol::EnsureScopeSlot(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo)
 {
-    if (this->NeedsSlotAlloc(funcInfo) && this->scopeSlot == Js::Constants::NoProperty)
+    if (this->NeedsSlotAlloc(byteCodeGenerator, funcInfo) && this->scopeSlot == Js::Constants::NoProperty)
     {
         this->scopeSlot = this->scope->AddScopeSlot();
     }

lib/Runtime/ByteCode/Symbol.h

@@ -480,9 +480,9 @@ class Symbol
         return this->name;
     }
 
-    Js::PropertyId EnsureScopeSlot(FuncInfo *funcInfo);
-    bool IsInSlot(FuncInfo *funcInfo, bool ensureSlotAlloc = false);
-    bool NeedsSlotAlloc(FuncInfo *funcInfo);
+    Js::PropertyId EnsureScopeSlot(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo);
+    bool IsInSlot(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool ensureSlotAlloc = false);
+    bool NeedsSlotAlloc(ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo);
 
     static void SaveToPropIdArray(Symbol *sym, Js::PropertyIdArray *propIds, ByteCodeGenerator *byteCodeGenerator, Js::PropertyId *pFirstSlot = nullptr);
 

lib/Runtime/Language/AsmJsModule.cpp

@@ -124,7 +124,7 @@ namespace Js
         if (funcInfo->byteCodeFunction->GetIsNamedFunctionExpression())
         {
             Assert(GetModuleFunctionNode()->sxFnc.pnodeName);
-            if (GetModuleFunctionNode()->sxFnc.pnodeName->sxVar.sym->IsInSlot(funcInfo))
+            if (GetModuleFunctionNode()->sxFnc.pnodeName->sxVar.sym->IsInSlot(GetByteCodeGenerator(), funcInfo))
             {
                 ParseNodePtr nameNode = GetModuleFunctionNode()->sxFnc.pnodeName;
                 GetByteCodeGenerator()->AssignPropertyId(nameNode->name());

lib/Runtime/Language/InterpreterStackFrame.cpp

@@ -1296,10 +1296,20 @@ namespace Js
             // In the debug mode zero out the local slot, so this could prevent locals being uninitialized in the case of setNextStatement.
             memset(newInstance->m_localSlots, 0, sizeof(Js::Var) * localCount);
         }
-        // Zero out only the return slot. This is not a user local, so the byte code will not initialize
-        // it to "undefined". And it's not an expression temp, so, for instance, a jitted loop body may expect
-        // it to be valid on entry to the loop, where "valid" means either a var or null.
-        newInstance->SetNonVarReg(0, NULL);
+        else 
+        {
+            Js::RegSlot varCount = function->GetFunctionBody()->GetVarCount();
+            if (varCount)
+            {
+                // Zero out the non-constant var slots.
+                Js::RegSlot constantCount = function->GetFunctionBody()->GetConstantCount();
+                memset(newInstance->m_localSlots + constantCount, 0, varCount * sizeof(Js::Var));
+            }
+            // Zero out the return slot. This is not a user local, so the byte code will not initialize
+            // it to "undefined". And it's not an expression temp, so, for instance, a jitted loop body may expect
+            // it to be valid on entry to the loop, where "valid" means either a var or null.
+            newInstance->SetNonVarReg(0, NULL);
+        }
 #endif
         // Wasm doesn't use const table
         if (!executeFunction->IsWasmFunction())