CVE-2020-1912: Out-of-bounds Write (CWE-787)
value: An out-of-bounds read/write vulnerability when executing lazily compiled inner generator functions in Facebook Hermes prior to commit 091835377369c8fd5917d9b87acffa721ad2a168 allows attackers to potentially execute arbitrary code via crafted JavaScript. Note that this is only exploitable if the application using Hermes permits evaluation of untrusted JavaScript. Hence, most React Native applications are not affected.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 8.1
Published: 2020-09-09T19:15Z
Repository: https://github.com/facebook/hermes
Language: C++
Files changed: include/hermes/IR/IR.h, include/hermes/IRGen/IRGen.h, lib/BCGen/HBC/BytecodeGenerator.cpp, lib/IRGen/ESTreeIRGen-func.cpp, lib/IRGen/ESTreeIRGen.cpp, test/BCGen/HBC/es6/generator.js, test/IRGen/es6/generator.js, test/IRGen/lazy-function-in-generator.js
Commit date: 2020-09-08 14:29:21 -0700
Commit message: Correctly restore whether or not a function is an inner generator

Summary:
If a generator was large enough to be lazily compiled, we would lose
that information when reconstituting the function's context. This meant
the function was generated as a regular function instead of a generator.

#utd-hermes-ignore-android

Reviewed By: tmikov

Differential Revision: D23580247

fbshipit-source-id: af5628bf322cbdc7c7cdfbb5f8d0756328518ea1

Diff: diff --git a/include/hermes/IR/IR.h b/include/hermes/IR/IR.h
index ec5d8f13c..47ca02ee3 100644
--- a/include/hermes/IR/IR.h
+++ b/include/hermes/IR/IR.h
@@ -334,12 +334,14 @@ class SerializedScope {
 };
 
 #ifndef HERMESVM_LEAN
 /// The source of a lazy AST node.
 struct LazySource {
-  // The type of node (such as a FunctionDeclaration or FunctionExpression).
+  /// The type of node (such as a FunctionDeclaration or FunctionExpression).
   ESTree::NodeKind nodeKind{ESTree::NodeKind::Empty};
+  /// Whether or not this is the inner function of a generator
+  bool isGeneratorInnerFunction;
   /// The source buffer id in which this function can be find.
   uint32_t bufferId{0};
   /// The range of the function within the buffer (the whole function node, not
   /// just the lazily parsed body).
   SMRange functionRange;
diff --git a/include/hermes/IRGen/IRGen.h b/include/hermes/IRGen/IRGen.h
index fb18b6fc5..f0e3cf1c8 100644
--- a/include/hermes/IRGen/IRGen.h
+++ b/include/hermes/IRGen/IRGen.h
@@ -40,10 +40,13 @@ struct LazyCompilationData {
   uint32_t bufferId;
 
   /// The type of function, e.g. statement or expression.
   ESTree::NodeKind nodeKind;
 
+  /// Whether or not this is the inner function of a generator.
+  bool isGeneratorInnerFunction;
+
   /// Whether or not the function is strict.
   bool strictMode;
 };
 } // namespace hbc
 
diff --git a/lib/BCGen/HBC/BytecodeGenerator.cpp b/lib/BCGen/HBC/BytecodeGenerator.cpp
index a3d150cf9..907f5c653 100644
--- a/lib/BCGen/HBC/BytecodeGenerator.cpp
+++ b/lib/BCGen/HBC/BytecodeGenerator.cpp
@@ -276,10 +276,12 @@ std::unique_ptr<BytecodeModule> BytecodeModuleGenerator::generate() {
       llvm_unreachable("Lazy support compiled out");
 #else
       auto lazyData = llvh::make_unique<LazyCompilationData>();
       lazyData->parentScope = F->getLazyScope();
       lazyData->nodeKind = F->getLazySource().nodeKind;
+      lazyData->isGeneratorInnerFunction =
+          F->getLazySource().isGeneratorInnerFunction;
       lazyData->bufferId = F->getLazySource().bufferId;
       lazyData->originalName = F->getOriginalOrInferredName();
       lazyData->closureAlias = F->getLazyClosureAlias()
           ? F->getLazyClosureAlias()->getName()
           : Identifier();
diff --git a/lib/IRGen/ESTreeIRGen-func.cpp b/lib/IRGen/ESTreeIRGen-func.cpp
index a5d568990..57d82a7e7 100644
--- a/lib/IRGen/ESTreeIRGen-func.cpp
+++ b/lib/IRGen/ESTreeIRGen-func.cpp
@@ -213,10 +213,11 @@ Function *ESTreeIRGen::genES5Function(
       // Set the AST position and variable context so we can continue later.
       newFunction->setLazyScope(saveCurrentScope());
       auto &lazySource = newFunction->getLazySource();
       lazySource.bufferId = bodyBlock->bufferId;
       lazySource.nodeKind = getLazyFunctionKind(functionNode);
+      lazySource.isGeneratorInnerFunction = isGeneratorInnerFunction;
       lazySource.functionRange = functionNode->getSourceRange();
 
       // Set the function's .length.
       newFunction->setExpectedParamCountIncludingThis(
           countExpectedArgumentsIncludingThis(functionNode));
@@ -300,18 +301,21 @@ Function *ESTreeIRGen::genGeneratorFunction(
       originalName,
       Function::DefinitionKind::ES5Function,
       ESTree::isStrict(functionNode->strictness),
       /* insertBefore */ nullptr);
 
-  auto *innerFn = genES5Function(
-      genAnonymousLabelName(originalName.isValid() ? originalName.str() : ""),
-      lazyClosureAlias,
-      functionNode,
-      true);
-
   {
     FunctionContext outerFnContext{this, outerFn, functionNode->getSemInfo()};
+
+    // Build the inner function. This must be done in the outerFnContext
+    // since it's lexically considered a child function.
+    auto *innerFn = genES5Function(
+        genAnonymousLabelName(originalName.isValid() ? originalName.str() : ""),
+        lazyClosureAlias,
+        functionNode,
+        true);
+
     emitFunctionPrologue(
         functionNode,
         Builder.createBasicBlock(outerFn),
         InitES5CaptureState::Yes,
         DoEmitParameters::No);
diff --git a/lib/IRGen/ESTreeIRGen.cpp b/lib/IRGen/ESTreeIRGen.cpp
index 1e0e77f66..0e8069d8d 100644
--- a/lib/IRGen/ESTreeIRGen.cpp
+++ b/lib/IRGen/ESTreeIRGen.cpp
@@ -347,11 +347,15 @@ std::pair<Function *, Function *> ESTreeIRGen::doLazyFunction(
 
   assert(
       !llvh::isa<ESTree::ArrowFunctionExpressionNode>(node) &&
       "lazy compilation not supported for arrow functions");
 
-  auto *func = genES5Function(lazyData->originalName, parentVar, node);
+  auto *func = genES5Function(
+      lazyData->originalName,
+      parentVar,
+      node,
+      lazyData->isGeneratorInnerFunction);
   addLexicalDebugInfo(func, topLevel, lexicalScopeChain);
   return {func, topLevel};
 }
 
 std::pair<Value *, bool> ESTreeIRGen::declareVariableOrGlobalProperty(
diff --git a/test/BCGen/HBC/es6/generator.js b/test/BCGen/HBC/es6/generator.js
index 2a2cf64a5..545b3468a 100644
--- a/test/BCGen/HBC/es6/generator.js
+++ b/test/BCGen/HBC/es6/generator.js
@@ -72,11 +72,11 @@ function *args() {
 // CHECK-LABEL: NCFunction<args>(1 params, 3 registers, 0 symbols):
 // CHECK-NEXT:     CreateEnvironment r0
 // CHECK-NEXT:     CreateGenerator   r1, r0, 4
 // CHECK-NEXT:     Ret               r1
 
-// CHECK-LABEL: Function<?anon_1_args>(1 params, 7 registers, 0 symbols):
+// CHECK-LABEL: Function<?anon_0_args>(1 params, 7 registers, 0 symbols):
 // CHECK-NEXT: Offset in debug table: source 0x{{.*}}, lexical 0x0000
 // CHECK-NEXT:     StartGenerator
 // CHECK-NEXT:     CreateEnvironment r0
 // CHECK-NEXT:     LoadConstUndefined r0
 // CHECK-NEXT:     LoadConstZero     r1
diff --git a/test/IRGen/es6/generator.js b/test/IRGen/es6/generator.js
index c69a0c52c..5d37e09cf 100644
--- a/test/IRGen/es6/generator.js
+++ b/test/IRGen/es6/generator.js
@@ -45,15 +45,15 @@ function *useResult() {
 }
 //CHECK-LABEL:function useResult()
 //CHECK-NEXT:frame = [x]
 //CHECK-NEXT:%BB0:
 //CHECK-NEXT:  %0 = StoreFrameInst undefined : undefined, [x]
-//CHECK-NEXT:  %1 = CreateGeneratorInst %?anon_1_useResult()
+//CHECK-NEXT:  %1 = CreateGeneratorInst %?anon_0_useResult()
 //CHECK-NEXT:  %2 = ReturnInst %1 : object
 //CHECK-NEXT:function_end
 
-//CHECK-LABEL:function ?anon_1_useResult()
+//CHECK-LABEL:function ?anon_0_useResult()
 //CHECK-NEXT:frame = [x]
 //CHECK-NEXT:%BB0:
 //CHECK-NEXT:  %0 = StartGeneratorInst
 //CHECK-NEXT:  %1 = AllocStackInst $?anon_0_isReturn_prologue
 //CHECK-NEXT:  %2 = ResumeGeneratorInst %1
@@ -84,15 +84,15 @@ function *loop(x) {
 }
 //CHECK-LABEL:function loop()
 //CHECK-NEXT:frame = [i]
 //CHECK-NEXT:%BB0:
 //CHECK-NEXT:  %0 = StoreFrameInst undefined : undefined, [i]
-//CHECK-NEXT:  %1 = CreateGeneratorInst %?anon_2_loop()
+//CHECK-NEXT:  %1 = CreateGeneratorInst %?anon_0_loop()
 //CHECK-NEXT:  %2 = ReturnInst %1 : object
 //CHECK-NEXT:function_end
 
-//CHECK-LABEL:function ?anon_2_loop(x)
+//CHECK-LABEL:function ?anon_0_loop(x)
 //CHECK-NEXT:frame = [i, x]
 //CHECK-NEXT:%BB0:
 //CHECK-NEXT:  %0 = StartGeneratorInst
 //CHECK-NEXT:  %1 = AllocStackInst $?anon_0_isReturn_prologue
 //CHECK-NEXT:  %2 = ResumeGeneratorInst %1
@@ -137,15 +137,15 @@ var simple2 = function*() {
   yield 1;
 }
 //CHECK-LABEL:function simple2()
 //CHECK-NEXT:frame = []
 //CHECK-NEXT:%BB0:
-//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_4_simple2()
+//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_0_simple2()
 //CHECK-NEXT:  %1 = ReturnInst %0 : object
 //CHECK-NEXT:function_end
 
-//CHECK-LABEL:function ?anon_4_simple2()
+//CHECK-LABEL:function ?anon_0_simple2()
 //CHECK-NEXT:frame = []
 //CHECK-NEXT:%BB0:
 //CHECK-NEXT:  %0 = StartGeneratorInst
 //CHECK-NEXT:  %1 = AllocStackInst $?anon_0_isReturn_prologue
 //CHECK-NEXT:  %2 = ResumeGeneratorInst %1
@@ -170,15 +170,15 @@ var yieldStar = function*() {
   yield* foo();
 }
 //CHECK-LABEL:function yieldStar()
 //CHECK-NEXT:frame = []
 //CHECK-NEXT:%BB0:
-//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_5_yieldStar()
+//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_0_yieldStar()
 //CHECK-NEXT:  %1 = ReturnInst %0 : object
 //CHECK-NEXT:function_end
 
-//CHECK-LABEL:function ?anon_5_yieldStar()
+//CHECK-LABEL:function ?anon_0_yieldStar()
 //CHECK-NEXT:frame = []
 //CHECK-NEXT:%BB0:
 //CHECK-NEXT:  %0 = StartGeneratorInst
 //CHECK-NEXT:  %1 = AllocStackInst $?anon_0_isReturn_prologue
 //CHECK-NEXT:  %2 = ResumeGeneratorInst %1
@@ -279,17 +279,17 @@ var destr = function*([x]) {
 }
 
 //CHECK-LABEL:function destr()
 //CHECK-NEXT:frame = []
 //CHECK-NEXT:%BB0:
-//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_6_destr()
+//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_0_destr()
 //CHECK-NEXT:  %1 = LoadPropertyInst %0 : object, "next" : string
 //CHECK-NEXT:  %2 = CallInst %1, %0 : object
 //CHECK-NEXT:  %3 = ReturnInst %0 : object
 //CHECK-NEXT:function_end
 
-//CHECK-LABEL:function ?anon_6_destr(?anon_2_param)
+//CHECK-LABEL:function ?anon_0_destr(?anon_2_param)
 //CHECK-NEXT:frame = [x]
 //CHECK-NEXT:%BB0:
 //CHECK-NEXT:  %0 = StartGeneratorInst
 //CHECK-NEXT:  %1 = AllocStackInst $?anon_0_isReturn_prologue
 //CHECK-NEXT:  %2 = ResumeGeneratorInst %1
@@ -353,17 +353,17 @@ var initializer = function*(x = foo()) {
 }
 
 //CHECK-LABEL:function initializer()
 //CHECK-NEXT:frame = []
 //CHECK-NEXT:%BB0:
-//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_7_initializer()
+//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_0_initializer()
 //CHECK-NEXT:  %1 = LoadPropertyInst %0 : object, "next" : string
 //CHECK-NEXT:  %2 = CallInst %1, %0 : object
 //CHECK-NEXT:  %3 = ReturnInst %0 : object
 //CHECK-NEXT:function_end
 
-//CHECK-LABEL:function ?anon_7_initializer(x)
+//CHECK-LABEL:function ?anon_0_initializer(x)
 //CHECK-NEXT:frame = [x]
 //CHECK-NEXT:%BB0:
 //CHECK-NEXT:  %0 = StartGeneratorInst
 //CHECK-NEXT:  %1 = AllocStackInst $?anon_0_isReturn_prologue
 //CHECK-NEXT:  %2 = ResumeGeneratorInst %1
diff --git a/test/IRGen/lazy-function-in-generator.js b/test/IRGen/lazy-function-in-generator.js
new file mode 100644
index 000000000..1eab92568
--- /dev/null
+++ b/test/IRGen/lazy-function-in-generator.js
@@ -0,0 +1,28 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hermes -lazy %s | %FileCheck %s --match-full-lines
+
+// Make sure we can correctly resolve scopes through lazily compiled
+// functions in lazily compiled generators.
+function f() {
+  var f_var = 10;
+  function* g() {
+    var g_var = 32;
+    function h() {
+      /* Some text to pad out the function so that it won't be eagerly compiled
+       * for being too short. Lorem ipsum dolor sit amet, consectetur adipiscing
+       * elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+       */
+      return f_var + g_var;
+    }
+    yield h();
+  }
+  return g().next().value;
+}
+// CHECK: 42
+print(f());

Vulnerable code:   // The type of node (such as a FunctionDeclaration or FunctionExpression).
  auto *innerFn = genES5Function(
      genAnonymousLabelName(originalName.isValid() ? originalName.str() : ""),
      lazyClosureAlias,
      functionNode,
      true);

  auto *func = genES5Function(lazyData->originalName, parentVar, node);
// CHECK-LABEL: Function<?anon_1_args>(1 params, 7 registers, 0 symbols):
//CHECK-NEXT:  %1 = CreateGeneratorInst %?anon_1_useResult()
//CHECK-LABEL:function ?anon_1_useResult()
//CHECK-NEXT:  %1 = CreateGeneratorInst %?anon_2_loop()
//CHECK-LABEL:function ?anon_2_loop(x)
//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_4_simple2()
//CHECK-LABEL:function ?anon_4_simple2()
//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_5_yieldStar()
//CHECK-LABEL:function ?anon_5_yieldStar()
//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_6_destr()
//CHECK-LABEL:function ?anon_6_destr(?anon_2_param)
//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_7_initializer()
//CHECK-LABEL:function ?anon_7_initializer(x)
Fixed code:   /// The type of node (such as a FunctionDeclaration or FunctionExpression).
  /// Whether or not this is the inner function of a generator
  bool isGeneratorInnerFunction;
  /// Whether or not this is the inner function of a generator.
  bool isGeneratorInnerFunction;

      lazyData->isGeneratorInnerFunction =
          F->getLazySource().isGeneratorInnerFunction;
      lazySource.isGeneratorInnerFunction = isGeneratorInnerFunction;

    // Build the inner function. This must be done in the outerFnContext
    // since it's lexically considered a child function.
    auto *innerFn = genES5Function(
        genAnonymousLabelName(originalName.isValid() ? originalName.str() : ""),
        lazyClosureAlias,
        functionNode,
        true);

  auto *func = genES5Function(
      lazyData->originalName,
      parentVar,
      node,
      lazyData->isGeneratorInnerFunction);
// CHECK-LABEL: Function<?anon_0_args>(1 params, 7 registers, 0 symbols):
//CHECK-NEXT:  %1 = CreateGeneratorInst %?anon_0_useResult()
//CHECK-LABEL:function ?anon_0_useResult()
//CHECK-NEXT:  %1 = CreateGeneratorInst %?anon_0_loop()
//CHECK-LABEL:function ?anon_0_loop(x)
//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_0_simple2()
//CHECK-LABEL:function ?anon_0_simple2()
//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_0_yieldStar()
//CHECK-LABEL:function ?anon_0_yieldStar()
//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_0_destr()
//CHECK-LABEL:function ?anon_0_destr(?anon_2_param)
//CHECK-NEXT:  %0 = CreateGeneratorInst %?anon_0_initializer()
//CHECK-LABEL:function ?anon_0_initializer(x)
/**
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

// RUN: %hermes -lazy %s | %FileCheck %s --match-full-lines

// Make sure we can correctly resolve scopes through lazily compiled
// functions in lazily compiled generators.
function f() {
  var f_var = 10;
  function* g() {
    var g_var = 32;
    function h() {
      /* Some text to pad out the function so that it won't be eagerly compiled
       * for being too short. Lorem ipsum dolor sit amet, consectetur adipiscing
       * elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
       */
      return f_var + g_var;
    }
    yield h();
  }
  return g().next().value;
}
// CHECK: 42
print(f());