Fix use-after-free on ambientDecls

Summary:
`ambientDecls` is currently passed to the `SemanticResolver` constructor
by reference which then stores the reference. Unfortunately, in a few
places (e.g. `resolveASTForParser`, `resolveASTLazy`) we pass in a
temporary for this parameter.

To fix this, pass it by pointer, and explicitly pass null when it is not
needed.

Reviewed By: avp

Differential Revision: D69624780

fbshipit-source-id: a312664682107aef004c88682939151fc9deb8d2

lib/Sema/SemResolve.cpp

@@ -169,7 +169,7 @@ bool resolveAST(
   SemanticResolver resolver{
       astContext,
       semCtx,
-      ambientDecls,
+      &ambientDecls,
       flowContext ? &declCollectorMap : nullptr,
       true,
       flowContext != nullptr};
@@ -197,7 +197,12 @@ bool resolveASTLazy(
     bool parentHadSuperBinding) {
   PerfSection validation("Resolving JavaScript lazy AST");
   // Resolve the entire AST.
-  SemanticResolver resolver{astContext, semCtx, {}, nullptr, true};
+  SemanticResolver resolver{
+      astContext,
+      semCtx,
+      /* ambientDecls */ nullptr,
+      /* saveDecls */ nullptr,
+      /* compile */ true};
   return resolver.runLazy(root, semInfo, parentHadSuperBinding);
 }
 
@@ -209,7 +214,12 @@ bool resolveASTInScope(
     bool parentHadSuperBinding) {
   PerfSection validation("Resolving JavaScript AST");
   // Resolve the entire AST.
-  SemanticResolver resolver{astContext, semCtx, {}, nullptr, true};
+  SemanticResolver resolver{
+      astContext,
+      semCtx,
+      /* ambientDecls */ nullptr,
+      /* saveDecls */ nullptr,
+      /* compile */ true};
   return resolver.runInScope(root, semInfo, parentHadSuperBinding);
 }
 
@@ -221,7 +231,11 @@ bool resolveCommonJSAST(
   PerfSection validation("Resolving JavaScript CommonJS Module AST");
   DeclCollectorMapTy declCollectorMap{};
   SemanticResolver resolver{
-      astContext, semCtx, {}, flowContext ? &declCollectorMap : nullptr, true};
+      astContext,
+      semCtx,
+      /* ambientDecls */ nullptr,
+      flowContext ? &declCollectorMap : nullptr,
+      /* compile */ true};
   if (!resolver.runCommonJSModule(root))
     return false;
 
@@ -276,7 +290,12 @@ bool resolveASTForParser(
     Context &astContext,
     SemContext &semCtx,
     ESTree::Node *root) {
-  SemanticResolver resolver{astContext, semCtx, {}, nullptr, false};
+  SemanticResolver resolver{
+      astContext,
+      semCtx,
+      /* ambientDecls */ nullptr,
+      /* saveDecls */ nullptr,
+      /* compile */ false};
   return resolver.run(llvh::cast<ESTree::ProgramNode>(root));
 }
 

lib/Sema/SemanticResolver.cpp

@@ -38,7 +38,7 @@ std::string stringForSHBuiltinError(
 SemanticResolver::SemanticResolver(
     Context &astContext,
     sema::SemContext &semCtx,
-    const DeclarationFileListTy &ambientDecls,
+    const DeclarationFileListTy *ambientDecls,
     DeclCollectorMapTy *saveDecls,
     bool compile,
     bool typed)
@@ -2367,6 +2367,9 @@ void SemanticResolver::processAmbientDecls() {
       globalScope_ &&
       "global scope must be created when declaring ambient globals");
 
+  if (!ambientDecls_)
+    return;
+
   /// This visitor structs collects declarations within a single closure without
   /// descending into child closures.
   struct DeclHoisting {
@@ -2421,7 +2424,7 @@ void SemanticResolver::processAmbientDecls() {
     }
   };
 
-  for (auto *programNode : ambientDecls_) {
+  for (auto *programNode : *ambientDecls_) {
     DeclHoisting DH;
     programNode->visit(DH);
     // Create variable declarations for each of the hoisted variables.

lib/Sema/SemanticResolver.h

@@ -43,9 +43,9 @@ class SemanticResolver
   /// Keywords we will be checking for.
   hermes::sema::Keywords kw_;
 
-  /// A list of parsed files containing global ambient declarations that should
-  /// be inserted in the global scope.
-  const DeclarationFileListTy &ambientDecls_;
+  /// If not null, a list of parsed files containing global ambient declarations
+  /// that should be inserted in the global scope.
+  const DeclarationFileListTy *ambientDecls_;
 
   /// A set of names that are restricted in the global scope.
   /// https://262.ecma-international.org/14.0/#sec-hasrestrictedglobalproperty
@@ -115,15 +115,15 @@ class SemanticResolver
   explicit SemanticResolver(
       Context &astContext,
       sema::SemContext &semCtx,
-      const DeclarationFileListTy &ambientDecls,
+      const DeclarationFileListTy *ambientDecls,
       DeclCollectorMapTy *saveDecls,
       bool compile,
       bool typed = false);
 
   explicit SemanticResolver(
       Context &astContext,
       sema::SemContext &semCtx,
-      const DeclarationFileListTy &ambientDecls,
+      const DeclarationFileListTy *ambientDecls,
       bool compile)
       : SemanticResolver(astContext, semCtx, ambientDecls, nullptr, compile) {}