[1.3>master] [MERGE #1979 @pleath] Change to address CVE-2016-7207

Merge pull request #1979 from pleath:release/1611.1.3 Fix incorrect identification of non-local-references when parsing arrow function parameter lists. Since we can't identify such syntax on the initial scan and have to backtrack and re-parse, our stacks of identifier references enter an inconsistent state that causes us to bind non-existent references and treat them as closure-captured. This in turn leads to bad byte code generation. The problem is easy to solve in the case of a single arrow function parameter not enclosed in parentheses. To handle the harder case of a list enclosed in parens, advance the current block ID when we begin parsing a parenthetical expression. Identifier references will be created that are amenable to the arrow function case, and if we do wind up with an arrow function, we only need to correct the function ID's on the references we've already made.
chakra-core · Nov 10, 2016 · 90e11e6 · 90e11e6
2 parents eede17b + b09833c
commit 90e11e6
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 14 deletions.
diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp
@@ -109,6 +109,7 @@ Parser::Parser(Js::ScriptContext* scriptContext, BOOL strictMode, PageAllocator
     m_nextFunctionId = nullptr;
     m_errorCallback = nullptr;
     m_uncertainStructure = FALSE;
+    m_reparsingLambdaParams = false;
     currBackgroundParseItem = nullptr;
     backgroundParseItems = nullptr;
     fastScannedRegExpNodes = nullptr;
@@ -813,6 +814,14 @@ Symbol* Parser::AddDeclForPid(ParseNodePtr pnode, IdentPtr pid, SymbolType symbo
     {
         Error(ERRnoMemory);
     }
+
+    if (refForDecl->funcId != GetCurrentFunctionNode()->sxFnc.functionId)
+    {
+        // Fix up the function id, which is incorrect if we're reparsing lambda parameters
+        Assert(this->m_reparsingLambdaParams);
+        refForDecl->funcId = GetCurrentFunctionNode()->sxFnc.functionId;
+    }
+
     if (blockInfo == GetCurrentBlockInfo())
     {
         refForUse = refForDecl;
@@ -2874,7 +2883,12 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
             }
         }
 
-        ref = this->PushPidRef(pid);
+        // Don't push a reference if this is a single lambda parameter, because we'll reparse with
+        // a correct function ID.
+        if (m_token.tk != tkDArrow)
+        {
+            ref = this->PushPidRef(pid);
+        }
 
         if (buildAST)
         {
@@ -2905,6 +2919,7 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
         break;
 
     case tkLParen:
+    {
         ichMin = m_pscan->IchMinTok();
         iuMin = m_pscan->IecpMinTok();
         m_pscan->Scan();
@@ -2928,11 +2943,27 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
             break;
         }
 
+        // Advance the block ID here in case this parenthetical expression turns out to be a lambda parameter list.
+        // That way the pid ref stacks will be created in their correct final form, and we can simply fix
+        // up function ID's.
+        uint saveNextBlockId = m_nextBlockId;
+        uint saveCurrBlockId = GetCurrentBlock()->sxBlock.blockId;
+        GetCurrentBlock()->sxBlock.blockId = m_nextBlockId++;
+
         this->m_parenDepth++;
         pnode = ParseExpr<buildAST>(koplNo, &fCanAssign, TRUE, FALSE, nullptr, nullptr /*nameLength*/, nullptr  /*pShortNameOffset*/, &term, true);
         this->m_parenDepth--;
 
         ChkCurTok(tkRParen, ERRnoRparen);
+
+        GetCurrentBlock()->sxBlock.blockId = saveCurrBlockId;
+        if (m_token.tk == tkDArrow)
+        {
+            // We're going to rewind and reinterpret the expression as a parameter list.
+            // Put back the original next-block-ID so the existing pid ref stacks will be correct.
+            m_nextBlockId = saveNextBlockId;
+        }
+
         // Emit a deferred ... error if one was parsed.
         if (m_deferEllipsisError && m_token.tk != tkDArrow)
         {
@@ -2944,6 +2975,7 @@ ParseNodePtr Parser::ParseTerm(BOOL fAllowCall,
             m_deferEllipsisError = false;
         }
         break;
+    }
 
     case tkIntCon:
         if (IsStrictMode() && m_pscan->IsOctOrLeadingZeroOnLastTKNumber())
@@ -4970,7 +5002,13 @@ bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, usho
 
         if (!skipFormals)
         {
+            bool fLambdaParamsSave = m_reparsingLambdaParams;
+            if (fLambda)
+            {
+                m_reparsingLambdaParams = true;
+            }
             this->ParseFncFormals<buildAST>(pnodeFnc, pnodeFncParent, flags);
+            m_reparsingLambdaParams = fLambdaParamsSave;
         }
 
         // Create function body scope
@@ -5035,22 +5073,17 @@ bool Parser::ParseFncDeclHelper(ParseNodePtr pnodeFnc, LPCOLESTR pNameHint, usho
             {
                 if (paramScope->GetCanMergeWithBodyScope())
                 {
-                    paramScope->ForEachSymbolUntil([this, paramScope](Symbol* sym) {
-                        if (sym->GetPid()->GetTopRef()->sym == nullptr)
+                    paramScope->ForEachSymbolUntil([this, paramScope, pnodeFnc](Symbol* sym) {
+                        if (sym->GetPid()->GetTopRef()->GetFuncScopeId() > pnodeFnc->sxFnc.functionId)
                         {
                             // One of the symbol has non local reference. Mark the param scope as we can't merge it with body scope.
                             paramScope->SetCannotMergeWithBodyScope();
                             return true;
                         }
-                        else
-                        {
-                            // If no non-local references are there then the top of the ref stack should point to the same symbol.
-                            Assert(sym->GetPid()->GetTopRef()->sym == sym);
-                        }
                         return false;
                     });
 
-                    if (wellKnownPropertyPids.arguments->GetTopRef() && wellKnownPropertyPids.arguments->GetTopRef()->GetScopeId() > pnodeFnc->sxFnc.pnodeScopes->sxBlock.blockId)
+                    if (wellKnownPropertyPids.arguments->GetTopRef() && wellKnownPropertyPids.arguments->GetTopRef()->GetFuncScopeId() > pnodeFnc->sxFnc.functionId)
                     {
                         Assert(pnodeFnc->sxFnc.UsesArguments());
                         // Arguments symbol is captured in the param scope
@@ -7981,12 +8014,8 @@ ParseNodePtr Parser::ParseExpr(int oplMin,
                 // binding operator, be it unary or binary.
                 Error(ERRsyntax);
             }
-            if (m_currentBlockInfo->pnodeBlock->sxBlock.blockType == PnodeBlockType::Parameter)
+            if (m_currentScope->GetScopeType() == ScopeType_Parameter)
             {
-                // 'yield' can appear (as a keyword) in parameter scope as formal name or as
-                // expression within a default parameter expression, in either a generator
-                // function or an arrow function contained within a generator function.
-                // In all cases it is an error.
                 Error(ERRsyntax);
             }
         }
@@ -8553,6 +8582,14 @@ PidRefStack* Parser::PushPidRef(IdentPtr pid)
         }
         pid->PushPidRef(blockId, funcId, ref);
     }
+    else if (m_reparsingLambdaParams)
+    {
+        // If we're reparsing params, then we may have pid refs left behind from the first pass. Make sure we're
+        // working with the right ref at this point.
+        ref = this->FindOrAddPidRef(pid, blockId, funcId);
+        // Fix up the function ID if we're reparsing lambda parameters.
+        ref->funcId = funcId;
+    }
 
     return ref;
 }

diff --git a/lib/Parser/Parse.h b/lib/Parser/Parse.h
@@ -367,6 +367,7 @@ class Parser
     ParseNodePtr * m_ppnodeVar;  // variable list tail
     bool m_inDeferredNestedFunc; // true if parsing a function in deferred mode, nested within the current node
     bool m_isInBackground;
+    bool m_reparsingLambdaParams;
 
     // This bool is used for deferring the shorthand initializer error ( {x = 1}) - as it is allowed in the destructuring grammar.
     bool m_hasDeferredShorthandInitError;

diff --git a/test/es6/lambda-params-shadow.js b/test/es6/lambda-params-shadow.js
@@ -0,0 +1,22 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+var count = 0;
+class A {
+    constructor() { count++; }
+    increment() { count++; }
+}
+class B extends A {
+    constructor() {
+        super();
+        ((B) => { super.increment() })();
+        (A=> { super.increment() })();
+    }
+}
+let b = new B();
+if (count !== 3) {
+    WScript.Echo('fail');
+}
+WScript.Echo('pass');
diff --git a/test/es6/rlexe.xml b/test/es6/rlexe.xml
@@ -12,6 +12,18 @@
       <compile-flags>-force:deferparse -args summary -endargs</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>lambda-params-shadow.js</files>
+      <compile-flags>-off:deferparse -args summary -endargs</compile-flags>
+    </default>
+  </test>
+  <test>
+    <default>
+      <files>lambda-params-shadow.js</files>
+      <compile-flags>-force:deferparse -args summary -endargs</compile-flags>
+    </default>
+  </test>
   <test>
     <default>
       <files>NumericLiteralTest.js</files>