From 089d030bc26b741ee2eb3c22d58f032c9e1c1a92 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 19 Nov 2021 11:08:25 +0100
Subject: [PATCH 1/7] make ApiLabel into a IPA type, and cache the public API
 of ApiGraphs

---
 .../ql/lib/semmle/javascript/ApiGraphs.qll    | 430 ++++++++++++------
 .../lib/semmle/javascript/frameworks/D3.qll   |   2 +-
 .../semmle/javascript/frameworks/History.qll  |   2 +-
 .../javascript/frameworks/Immutable.qll       |   2 +-
 .../semmle/javascript/frameworks/Logging.qll  |   2 +-
 .../lib/semmle/javascript/frameworks/Nest.qll |   2 +-
 .../semmle/javascript/frameworks/Redux.qll    |   4 +-
 .../lib/semmle/javascript/frameworks/Vue.qll  |   4 +-
 .../javascript/internal/CachedStages.qll      |  37 ++
 ...APIUsedWithUntrustedDataCustomizations.qll |   8 +-
 .../security/dataflow/RemoteFlowSources.qll   |  15 +-
 .../ql/src/meta/ApiGraphs/ApiGraphEdges.ql    |   2 +-
 .../ql/test/ApiGraphs/VerifyAssertions.qll    |   9 +-
 13 files changed, 352 insertions(+), 167 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
index c3083d57d9d..740ac525b54 100644
--- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
+++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
@@ -11,6 +11,7 @@
 
 import javascript
 private import semmle.javascript.dataflow.internal.FlowSteps as FlowSteps
+private import internal.CachedStages
 
 /**
  * Provides classes and predicates for working with APIs defined or used in a database.
@@ -33,6 +34,7 @@ module API {
      * As another example, in the assignment `exports.plusOne = (x) => x+1` the two references to
      * `x` are uses of the first parameter of `plusOne`.
      */
+    pragma[inline]
     DataFlow::Node getAUse() {
       exists(DataFlow::SourceNode src | Impl::use(this, src) |
         Impl::trackUseNode(src).flowsTo(result)
@@ -105,22 +107,31 @@ module API {
      * For example, modules have an `exports` member representing their exports, and objects have
      * their properties as members.
      */
-    bindingset[m]
-    bindingset[result]
-    Node getMember(string m) { result = getASuccessor(Label::member(m)) }
+    cached
+    Node getMember(string m) {
+      Stages::APIStage::ref() and
+      result = getASuccessor(Label::member(m))
+    }
 
     /**
      * Gets a node representing a member of this API component where the name of the member is
      * not known statically.
      */
-    Node getUnknownMember() { result = getASuccessor(Label::unknownMember()) }
+    cached
+    Node getUnknownMember() {
+      Stages::APIStage::ref() and
+      result = getASuccessor(Label::unknownMember())
+    }
 
     /**
      * Gets a node representing a member of this API component where the name of the member may
      * or may not be known statically.
      */
+    cached
     Node getAMember() {
-      result = getASuccessor(Label::member(_)) or
+      Stages::APIStage::ref() and
+      result = getMember(_)
+      or
       result = getUnknownMember()
     }
 
@@ -135,7 +146,11 @@ module API {
      * This predicate may have multiple results when there are multiple constructor calls invoking this API component.
      * Consider using `getAnInstantiation()` if there is a need to distinguish between individual constructor calls.
      */
-    Node getInstance() { result = getASuccessor(Label::instance()) }
+    cached
+    Node getInstance() {
+      Stages::APIStage::ref() and
+      result = getASuccessor(Label::instance())
+    }
 
     /**
      * Gets a node representing the `i`th parameter of the function represented by this node.
@@ -143,16 +158,16 @@ module API {
      * This predicate may have multiple results when there are multiple invocations of this API component.
      * Consider using `getAnInvocation()` if there is a need to distingiush between individual calls.
      */
-    bindingset[i]
-    Node getParameter(int i) { result = getASuccessor(Label::parameter(i)) }
+    cached
+    Node getParameter(int i) {
+      Stages::APIStage::ref() and
+      result = getASuccessor(Label::parameter(i))
+    }
 
     /**
      * Gets the number of parameters of the function represented by this node.
      */
-    int getNumParameter() {
-      result =
-        max(string s | exists(getASuccessor(Label::parameterByStringIndex(s))) | s.toInt()) + 1
-    }
+    int getNumParameter() { result = max(int s | exists(getParameter(s))) + 1 }
 
     /**
      * Gets a node representing the last parameter of the function represented by this node.
@@ -165,7 +180,11 @@ module API {
     /**
      * Gets a node representing the receiver of the function represented by this node.
      */
-    Node getReceiver() { result = getASuccessor(Label::receiver()) }
+    cached
+    Node getReceiver() {
+      Stages::APIStage::ref() and
+      result = getASuccessor(Label::receiver())
+    }
 
     /**
      * Gets a node representing a parameter or the receiver of the function represented by this
@@ -175,8 +194,11 @@ module API {
      * there are multiple invocations of this API component.
      * Consider using `getAnInvocation()` if there is a need to distingiush between individual calls.
      */
+    cached
     Node getAParameter() {
-      result = getASuccessor(Label::parameterByStringIndex(_)) or
+      Stages::APIStage::ref() and
+      result = getParameter(_)
+      or
       result = getReceiver()
     }
 
@@ -186,18 +208,30 @@ module API {
      * This predicate may have multiple results when there are multiple invocations of this API component.
      * Consider using `getACall()` if there is a need to distingiush between individual calls.
      */
-    Node getReturn() { result = getASuccessor(Label::return()) }
+    cached
+    Node getReturn() {
+      Stages::APIStage::ref() and
+      result = getASuccessor(Label::return())
+    }
 
     /**
      * Gets a node representing the promised value wrapped in the `Promise` object represented by
      * this node.
      */
-    Node getPromised() { result = getASuccessor(Label::promised()) }
+    cached
+    Node getPromised() {
+      Stages::APIStage::ref() and
+      result = getASuccessor(Label::promised())
+    }
 
     /**
      * Gets a node representing the error wrapped in the `Promise` object represented by this node.
      */
-    Node getPromisedError() { result = getASuccessor(Label::promisedError()) }
+    cached
+    Node getPromisedError() {
+      Stages::APIStage::ref() and
+      result = getASuccessor(Label::promisedError())
+    }
 
     /**
      * Gets a string representation of the lexicographically least among all shortest access paths
@@ -209,13 +243,13 @@ module API {
      * Gets a node such that there is an edge in the API graph between this node and the other
      * one, and that edge is labeled with `lbl`.
      */
-    Node getASuccessor(string lbl) { Impl::edge(this, lbl, result) }
+    Node getASuccessor(ApiLabel lbl) { Impl::edge(this, lbl, result) }
 
     /**
      * Gets a node such that there is an edge in the API graph between that other node and
      * this one, and that edge is labeled with `lbl`
      */
-    Node getAPredecessor(string lbl) { this = result.getASuccessor(lbl) }
+    Node getAPredecessor(ApiLabel lbl) { this = result.getASuccessor(lbl) }
 
     /**
      * Gets a node such that there is an edge in the API graph between this node and the other
@@ -281,9 +315,9 @@ module API {
       length = 0 and
       result = ""
       or
-      exists(Node pred, string lbl, string predpath |
+      exists(Node pred, ApiLabel lbl, string predpath |
         Impl::edge(pred, lbl, this) and
-        lbl != "" and
+        not lbl instanceof Label::LabelAlias and
         predpath = pred.getAPath(length - 1) and
         exists(string space | if length = 1 then space = "" else space = " " |
           result = "(" + lbl + space + predpath + ")" and
@@ -348,6 +382,11 @@ module API {
 
     /** Gets a data-flow node that defines this entry point. */
     abstract DataFlow::Node getARhs();
+
+    /** Gets an API-node for this entry point. */
+    API::Node getNode() {
+      result = root().getASuccessor(any(Label::LabelEntryPoint l | l.getEntryPoint() = this))
+    }
   }
 
   /**
@@ -431,27 +470,16 @@ module API {
       hasSemantics(imp)
     }
 
-    /** Gets the definition of module `m`. */
-    private Module importableModule(string m) {
-      exists(NPMPackage pkg, PackageJSON json |
-        json = pkg.getPackageJSON() and not json.isPrivate()
-      |
-        result = pkg.getMainModule() and
-        not result.isExterns() and
-        m = pkg.getPackageName()
-      )
-    }
-
     /**
      * Holds if `rhs` is the right-hand side of a definition of a node that should have an
      * incoming edge from `base` labeled `lbl` in the API graph.
      */
     cached
-    predicate rhs(TApiNode base, string lbl, DataFlow::Node rhs) {
+    predicate rhs(TApiNode base, ApiLabel lbl, DataFlow::Node rhs) {
       hasSemantics(rhs) and
       (
         base = MkRoot() and
-        rhs = lbl.(EntryPoint).getARhs()
+        rhs = lbl.(Label::LabelEntryPoint).getEntryPoint().getARhs()
         or
         exists(string m, string prop |
           base = MkModuleExport(m) and
@@ -563,7 +591,7 @@ module API {
      */
     pragma[noinline]
     private predicate propertyRead(
-      DataFlow::SourceNode pred, string propDesc, string lbl, DataFlow::Node ref
+      DataFlow::SourceNode pred, string propDesc, ApiLabel lbl, DataFlow::Node ref
     ) {
       ref = pred.getAPropertyRead() and
       lbl = Label::memberFromRef(ref) and
@@ -587,11 +615,11 @@ module API {
      * `lbl` in the API graph.
      */
     cached
-    predicate use(TApiNode base, string lbl, DataFlow::Node ref) {
+    predicate use(TApiNode base, ApiLabel lbl, DataFlow::Node ref) {
       hasSemantics(ref) and
       (
         base = MkRoot() and
-        ref = lbl.(EntryPoint).getAUse()
+        ref = lbl.(Label::LabelEntryPoint).getEntryPoint().getAUse()
         or
         // property reads
         exists(DataFlow::SourceNode src, DataFlow::SourceNode pred, string propDesc |
@@ -678,33 +706,6 @@ module API {
       nd = MkUse(ref)
     }
 
-    /** Holds if module `m` exports `rhs`. */
-    private predicate exports(string m, DataFlow::Node rhs) {
-      exists(Module mod | mod = importableModule(m) |
-        rhs = mod.(AmdModule).getDefine().getModuleExpr().flow()
-        or
-        exports(m, "default", rhs)
-        or
-        exists(ExportAssignDeclaration assgn | assgn.getTopLevel() = mod |
-          rhs = assgn.getExpression().flow()
-        )
-        or
-        rhs = mod.(Closure::ClosureModule).getExportsVariable().getAnAssignedExpr().flow()
-      )
-    }
-
-    /** Holds if module `m` exports `rhs` under the name `prop`. */
-    private predicate exports(string m, string prop, DataFlow::Node rhs) {
-      exists(ExportDeclaration exp | exp.getEnclosingModule() = importableModule(m) |
-        rhs = exp.getSourceNode(prop)
-        or
-        exists(Variable v |
-          exp.exportsAs(v, prop) and
-          rhs = v.getAnAssignedExpr().flow()
-        )
-      )
-    }
-
     private import semmle.javascript.dataflow.TypeTracking
 
     /**
@@ -863,7 +864,8 @@ module API {
      * Holds if there is an edge from `pred` to `succ` in the API graph that is labeled with `lbl`.
      */
     cached
-    predicate edge(TApiNode pred, string lbl, TApiNode succ) {
+    predicate edge(TApiNode pred, ApiLabel lbl, TApiNode succ) {
+      Stages::APIStage::ref() and
       exists(string m |
         pred = MkRoot() and
         lbl = Label::mod(m)
@@ -970,6 +972,7 @@ module API {
     /**
      * Gets an API node where a RHS of the node is the `i`th argument to this call.
      */
+    pragma[noinline]
     private Node getAParameterCandidate(int i) { result.getARhs() = getArgument(i) }
 
     /** Gets the API node for a parameter of this invocation. */
@@ -996,89 +999,224 @@ module API {
 
   /** A `new` call connected to the API graph. */
   class NewNode extends InvokeNode, DataFlow::NewNode { }
+
+  /** A label in the API-graph */
+  abstract class ApiLabel extends Label::TLabel {
+    string toString() { result = "???" }
+  }
+
+  private module Label {
+    newtype TLabel =
+      MkLabelMod(string mod) {
+        exists(Impl::MkModuleExport(mod)) or
+        exists(Impl::MkModuleImport(mod))
+      } or
+      MkLabelInstance() or
+      MkLabelMember(string prop) {
+        exports(_, prop, _) or
+        exists(any(DataFlow::ClassNode c).getInstanceMethod(prop)) or
+        prop = "exports" or
+        prop = any(CanonicalName c).getName() or
+        prop = any(DataFlow::PropRef p).getPropertyName() or
+        exists(Impl::MkTypeUse(_, prop)) or
+        exists(any(Module m).getAnExportedValue(prop))
+      } or
+      MkLabelUnknownMember() or
+      MkLabelParameter(int i) {
+        i =
+          [-1 .. max(int args |
+              args = any(InvokeExpr invk).getNumArgument() or
+              args = any(Function f).getNumParameter()
+            )] or
+        i = [0 .. 10]
+      } or
+      MkLabelReturn() or
+      MkLabelPromised() or
+      MkLabelPromisedError() or
+      MkLabelAlias() or
+      MkLabelEntryPoint(API::EntryPoint e)
+
+    class LabelEntryPoint extends ApiLabel {
+      API::EntryPoint e;
+
+      LabelEntryPoint() { this = MkLabelEntryPoint(e) }
+
+      API::EntryPoint getEntryPoint() { result = e }
+
+      override string toString() { result = e }
+    }
+
+    class LabelAlias extends ApiLabel {
+      LabelAlias() { this = MkLabelAlias() }
+
+      override string toString() { result = "" }
+    }
+
+    class LabelPromised extends ApiLabel {
+      LabelPromised() { this = MkLabelPromised() }
+
+      override string toString() { result = "promised" }
+    }
+
+    class LabelPromisedError extends ApiLabel {
+      LabelPromisedError() { this = MkLabelPromisedError() }
+
+      override string toString() { result = "promised" }
+    }
+
+    class LabelReturn extends ApiLabel {
+      LabelReturn() { this = MkLabelReturn() }
+
+      override string toString() { result = "return" }
+    }
+
+    class LabelMod extends ApiLabel {
+      string mod;
+
+      LabelMod() { this = MkLabelMod(mod) }
+
+      string getMod() { result = mod }
+
+      override string toString() { result = "module " + mod }
+    }
+
+    class LabelInstance extends ApiLabel {
+      LabelInstance() { this = MkLabelInstance() }
+
+      override string toString() { result = "instance" }
+    }
+
+    class LabelMember extends ApiLabel {
+      string prop;
+
+      LabelMember() { this = MkLabelMember(prop) }
+
+      string getProperty() { result = prop }
+
+      override string toString() { result = "member " + prop }
+    }
+
+    class LabelUnknownMember extends ApiLabel {
+      LabelUnknownMember() { this = MkLabelUnknownMember() }
+
+      override string toString() { result = "member *" }
+    }
+
+    class LabelParameter extends ApiLabel {
+      int i;
+
+      LabelParameter() { this = MkLabelParameter(i) }
+
+      override string toString() { result = "parameter " + i }
+
+      int getIndex() { result = i }
+    }
+
+    /** Gets the edge label for the module `m`. */
+    LabelMod mod(string m) { result.getMod() = m }
+
+    /** Gets the `member` edge label for member `m`. */
+    bindingset[m]
+    bindingset[result]
+    LabelMember member(string m) { result.getProperty() = m }
+
+    /** Gets the `member` edge label for the unknown member. */
+    LabelUnknownMember unknownMember() { any() }
+
+    /**
+     * Gets a property name referred to by the given dynamic property access,
+     * allowing one property flow step in the process (to allow flow through imports).
+     *
+     * This is to support code patterns where the property name is actually constant,
+     * but the property name has been factored into a library.
+     */
+    private string getAnIndirectPropName(DataFlow::PropRef ref) {
+      exists(DataFlow::Node pred |
+        FlowSteps::propertyFlowStep(pred, ref.getPropertyNameExpr().flow()) and
+        result = pred.getStringValue()
+      )
+    }
+
+    /**
+     * Gets unique result of `getAnIndirectPropName` if there is one.
+     */
+    private string getIndirectPropName(DataFlow::PropRef ref) {
+      result = unique(string s | s = getAnIndirectPropName(ref))
+    }
+
+    /** Gets the `member` edge label for the given property reference. */
+    ApiLabel memberFromRef(DataFlow::PropRef pr) {
+      exists(string pn | pn = pr.getPropertyName() or pn = getIndirectPropName(pr) |
+        result = member(pn) and
+        // only consider properties with alphanumeric(-ish) names, excluding special properties
+        // and properties whose names look like they are meant to be internal
+        pn.regexpMatch("(?!prototype$|__)[\\w_$][\\w\\-.$]*")
+      )
+      or
+      not exists(pr.getPropertyName()) and
+      not exists(getIndirectPropName(pr)) and
+      result = unknownMember()
+    }
+
+    /** Gets the `instance` edge label. */
+    LabelInstance instance() { any() }
+
+    /**
+     * Gets the `parameter` edge label for the `i`th parameter.
+     *
+     * The receiver is considered to be parameter -1.
+     */
+    LabelParameter parameter(int i) { result.getIndex() = i }
+
+    /** Gets the `parameter` edge label for the receiver. */
+    LabelParameter receiver() { result = parameter(-1) }
+
+    /** Gets the `return` edge label. */
+    LabelReturn return() { any() }
+
+    /** Gets the `alias` (empty) edge label. */
+    LabelAlias alias() { any() }
+
+    /** Gets the `promised` edge label connecting a promise to its contained value. */
+    MkLabelPromised promised() { any() }
+
+    /** Gets the `promisedError` edge label connecting a promise to its rejected value. */
+    MkLabelPromisedError promisedError() { any() }
+  }
 }
 
-private module Label {
-  /** Gets the edge label for the module `m`. */
-  bindingset[m]
-  bindingset[result]
-  string mod(string m) { result = "module " + m }
-
-  /** Gets the `member` edge label for member `m`. */
-  bindingset[m]
-  bindingset[result]
-  string member(string m) { result = "member " + m }
-
-  /** Gets the `member` edge label for the unknown member. */
-  string unknownMember() { result = "member *" }
-
-  /**
-   * Gets a property name referred to by the given dynamic property access,
-   * allowing one property flow step in the process (to allow flow through imports).
-   *
-   * This is to support code patterns where the property name is actually constant,
-   * but the property name has been factored into a library.
-   */
-  private string getAnIndirectPropName(DataFlow::PropRef ref) {
-    exists(DataFlow::Node pred |
-      FlowSteps::propertyFlowStep(pred, ref.getPropertyNameExpr().flow()) and
-      result = pred.getStringValue()
-    )
-  }
-
-  /**
-   * Gets unique result of `getAnIndirectPropName` if there is one.
-   */
-  private string getIndirectPropName(DataFlow::PropRef ref) {
-    result = unique(string s | s = getAnIndirectPropName(ref))
-  }
-
-  /** Gets the `member` edge label for the given property reference. */
-  string memberFromRef(DataFlow::PropRef pr) {
-    exists(string pn | pn = pr.getPropertyName() or pn = getIndirectPropName(pr) |
-      result = member(pn) and
-      // only consider properties with alphanumeric(-ish) names, excluding special properties
-      // and properties whose names look like they are meant to be internal
-      pn.regexpMatch("(?!prototype$|__)[\\w_$][\\w\\-.$]*")
+/** Holds if module `m` exports `rhs`. */
+private predicate exports(string m, DataFlow::Node rhs) {
+  exists(Module mod | mod = importableModule(m) |
+    rhs = mod.(AmdModule).getDefine().getModuleExpr().flow()
+    or
+    exports(m, "default", rhs)
+    or
+    exists(ExportAssignDeclaration assgn | assgn.getTopLevel() = mod |
+      rhs = assgn.getExpression().flow()
     )
     or
-    not exists(pr.getPropertyName()) and
-    not exists(getIndirectPropName(pr)) and
-    result = unknownMember()
-  }
-
-  /** Gets the `instance` edge label. */
-  string instance() { result = "instance" }
-
-  /**
-   * Gets the `parameter` edge label for the parameter `s`.
-   *
-   * This is an internal helper predicate; use `parameter` instead.
-   */
-  bindingset[result]
-  bindingset[s]
-  string parameterByStringIndex(string s) {
-    result = "parameter " + s and
-    s.toInt() >= -1
-  }
-
-  /**
-   * Gets the `parameter` edge label for the `i`th parameter.
-   *
-   * The receiver is considered to be parameter -1.
-   */
-  bindingset[i]
-  string parameter(int i) { result = parameterByStringIndex(i.toString()) }
-
-  /** Gets the `parameter` edge label for the receiver. */
-  string receiver() { result = "parameter -1" }
-
-  /** Gets the `return` edge label. */
-  string return() { result = "return" }
-
-  /** Gets the `promised` edge label connecting a promise to its contained value. */
-  string promised() { result = "promised" }
-
-  /** Gets the `promisedError` edge label connecting a promise to its rejected value. */
-  string promisedError() { result = "promisedError" }
+    rhs = mod.(Closure::ClosureModule).getExportsVariable().getAnAssignedExpr().flow()
+  )
+}
+
+/** Holds if module `m` exports `rhs` under the name `prop`. */
+private predicate exports(string m, string prop, DataFlow::Node rhs) {
+  exists(ExportDeclaration exp | exp.getEnclosingModule() = importableModule(m) |
+    rhs = exp.getSourceNode(prop)
+    or
+    exists(Variable v |
+      exp.exportsAs(v, prop) and
+      rhs = v.getAnAssignedExpr().flow()
+    )
+  )
+}
+
+/** Gets the definition of module `m`. */
+private Module importableModule(string m) {
+  exists(NPMPackage pkg, PackageJSON json | json = pkg.getPackageJSON() and not json.isPrivate() |
+    result = pkg.getMainModule() and
+    not result.isExterns() and
+    m = pkg.getPackageName()
+  )
 }
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/D3.qll b/javascript/ql/lib/semmle/javascript/frameworks/D3.qll
index 49ec68e2424..252e87d3c1d 100644
--- a/javascript/ql/lib/semmle/javascript/frameworks/D3.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/D3.qll
@@ -23,7 +23,7 @@ module D3 {
     or
     result = API::moduleImport("d3-node").getInstance().getMember("d3")
     or
-    result = API::root().getASuccessor(any(D3GlobalEntry i))
+    result = any(D3GlobalEntry i).getNode()
   }
 
   /**
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/History.qll b/javascript/ql/lib/semmle/javascript/frameworks/History.qll
index 05be7ec36dd..6913fe93daf 100644
--- a/javascript/ql/lib/semmle/javascript/frameworks/History.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/History.qll
@@ -17,7 +17,7 @@ module History {
    * Gets a reference to the [`history`](https://npmjs.org/package/history) library.
    */
   private API::Node history() {
-    result = [API::moduleImport("history"), API::root().getASuccessor(any(HistoryGlobalEntry h))]
+    result = [API::moduleImport("history"), any(HistoryGlobalEntry h).getNode()]
   }
 
   /**
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Immutable.qll b/javascript/ql/lib/semmle/javascript/frameworks/Immutable.qll
index c039199bbd4..6afb60f73d3 100644
--- a/javascript/ql/lib/semmle/javascript/frameworks/Immutable.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/Immutable.qll
@@ -27,7 +27,7 @@ private module Immutable {
   API::Node immutableImport() {
     result = API::moduleImport("immutable")
     or
-    result = API::root().getASuccessor(any(ImmutableGlobalEntry i))
+    result = any(ImmutableGlobalEntry i).getNode()
   }
 
   /**
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Logging.qll b/javascript/ql/lib/semmle/javascript/frameworks/Logging.qll
index 4a64ed2a7e1..ea8f97ca934 100644
--- a/javascript/ql/lib/semmle/javascript/frameworks/Logging.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/Logging.qll
@@ -45,7 +45,7 @@ private module Console {
    */
   private API::Node console() {
     result = API::moduleImport("console") or
-    result = API::root().getASuccessor(any(ConsoleGlobalEntry e))
+    result = any(ConsoleGlobalEntry e).getNode()
   }
 
   /**
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Nest.qll b/javascript/ql/lib/semmle/javascript/frameworks/Nest.qll
index d216fac1712..88d626e5398 100644
--- a/javascript/ql/lib/semmle/javascript/frameworks/Nest.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/Nest.qll
@@ -151,7 +151,7 @@ module NestJS {
   private API::Node validationPipe() {
     result = nestjs().getMember("ValidationPipe")
     or
-    result = API::root().getASuccessor(any(ValidationNodeEntry e))
+    result = any(ValidationNodeEntry e).getNode()
   }
 
   /**
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Redux.qll b/javascript/ql/lib/semmle/javascript/frameworks/Redux.qll
index d26982cef7f..1451c69ada8 100644
--- a/javascript/ql/lib/semmle/javascript/frameworks/Redux.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/Redux.qll
@@ -1111,9 +1111,7 @@ module Redux {
 
     /** A heuristic call to `connect`, recognized by it taking arguments named `mapStateToProps` and `mapDispatchToProps`. */
     private class HeuristicConnectFunction extends ConnectCall {
-      HeuristicConnectFunction() {
-        this = API::root().getASuccessor(any(HeuristicConnectEntryPoint e)).getACall()
-      }
+      HeuristicConnectFunction() { this = any(HeuristicConnectEntryPoint e).getNode().getACall() }
 
       override API::Node getMapStateToProps() {
         result = getAParameter() and
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Vue.qll b/javascript/ql/lib/semmle/javascript/frameworks/Vue.qll
index 2101f29fdab..8689140f126 100644
--- a/javascript/ql/lib/semmle/javascript/frameworks/Vue.qll
+++ b/javascript/ql/lib/semmle/javascript/frameworks/Vue.qll
@@ -35,7 +35,7 @@ module Vue {
   API::Node vueLibrary() {
     result = API::moduleImport("vue")
     or
-    result = API::root().getASuccessor(any(GlobalVueEntryPoint e))
+    result = any(GlobalVueEntryPoint e).getNode()
   }
 
   /**
@@ -51,7 +51,7 @@ module Vue {
     or
     result = vueLibrary().getMember("component").getReturn()
     or
-    result = API::root().getASuccessor(any(VueFileImportEntryPoint e))
+    result = any(VueFileImportEntryPoint e).getNode()
   }
 
   /**
diff --git a/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll b/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
index e2df38f0c7c..da7658fe872 100644
--- a/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
+++ b/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
@@ -233,6 +233,43 @@ module Stages {
     }
   }
 
+  /**
+   * The `APIStage` stage.
+   */
+  cached
+  module APIStage {
+    /**
+     * Always holds.
+     * Ensures that a predicate is evaluated as part of the APIStage stage.
+     */
+    cached
+    predicate ref() { 1 = 1 }
+
+    /**
+     * DONT USE!
+     * Contains references to each predicate that use the above `ref` predicate.
+     */
+    cached
+    predicate backref() {
+      1 = 1
+      or
+      exists(
+        API::moduleImport("foo")
+            .getMember("bar")
+            .getUnknownMember()
+            .getAMember()
+            .getAParameter()
+            .getPromised()
+            .getReturn()
+            .getParameter(2)
+            .getUnknownMember()
+            .getInstance()
+            .getReceiver()
+            .getPromisedError()
+      )
+    }
+  }
+
   /**
    * The `taint` stage.
    */
diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll
index f9eaa7c7ce7..c2a3736e778 100644
--- a/javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll
+++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll
@@ -211,11 +211,9 @@ module ExternalAPIUsedWithUntrustedData {
         node = getNamedParameter(base.getAParameter(), paramName) and
         result = basename + ".[callback].[param '" + paramName + "']"
         or
-        exists(string callbackName, string index |
-          node =
-            getNamedParameter(base.getASuccessor("parameter " + index).getMember(callbackName),
-              paramName) and
-          index != "-1" and // ignore receiver
+        exists(string callbackName, int index |
+          node = getNamedParameter(base.getParameter(index).getMember(callbackName), paramName) and
+          index != -1 and // ignore receiver
           result =
             basename + ".[callback " + index + " '" + callbackName + "'].[param '" + paramName +
               "']"
diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll
index 89574f4886b..81fb8dd2cf5 100644
--- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll
+++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll
@@ -117,16 +117,23 @@ private class RemoteFlowSourceAccessPath extends JSONString {
   string getSourceType() { result = sourceType }
 
   /** Gets the `i`th component of the access path specifying this remote flow source. */
-  string getComponent(int i) {
+  API::ApiLabel getComponent(int i) {
     exists(string raw | raw = this.getValue().splitAt(".", i + 1) |
       i = 0 and
-      result = "ExternalRemoteFlowSourceSpec " + raw
+      result
+          .(API::EdgeLabel::LabelEntryPoint)
+          .getEntryPoint()
+          .(ExternalRemoteFlowSourceSpecEntryPoint)
+          .getName() = raw
       or
       i > 0 and
       result = API::EdgeLabel::member(raw)
     )
   }
 
+  /** Gets the first part of this access path. E.g. for "window.user.name" the result is "window". */
+  string getRootPath() { result = this.getValue().splitAt(".", 1) }
+
   /** Gets the index of the last component of this access path. */
   int getMaxComponentIndex() { result = max(int i | exists(getComponent(i))) }
 
@@ -154,10 +161,12 @@ private class ExternalRemoteFlowSourceSpecEntryPoint extends API::EntryPoint {
   string name;
 
   ExternalRemoteFlowSourceSpecEntryPoint() {
-    this = any(RemoteFlowSourceAccessPath s).getComponent(0) and
+    name = any(RemoteFlowSourceAccessPath s).getRootPath() and
     this = "ExternalRemoteFlowSourceSpec " + name
   }
 
+  string getName() { result = name }
+
   override DataFlow::SourceNode getAUse() { result = DataFlow::globalVarRef(name) }
 
   override DataFlow::Node getARhs() { none() }
diff --git a/javascript/ql/src/meta/ApiGraphs/ApiGraphEdges.ql b/javascript/ql/src/meta/ApiGraphs/ApiGraphEdges.ql
index 16718a9d122..1920bfb183f 100644
--- a/javascript/ql/src/meta/ApiGraphs/ApiGraphEdges.ql
+++ b/javascript/ql/src/meta/ApiGraphs/ApiGraphEdges.ql
@@ -12,4 +12,4 @@ import javascript
 import meta.MetaMetrics
 
 select projectRoot(),
-  count(API::Node pred, string lbl, API::Node succ | succ = pred.getASuccessor(lbl))
+  count(API::Node pred, API::ApiLabel lbl, API::Node succ | succ = pred.getASuccessor(lbl))
diff --git a/javascript/ql/test/ApiGraphs/VerifyAssertions.qll b/javascript/ql/test/ApiGraphs/VerifyAssertions.qll
index 4c92792cce5..6a57d85518a 100644
--- a/javascript/ql/test/ApiGraphs/VerifyAssertions.qll
+++ b/javascript/ql/test/ApiGraphs/VerifyAssertions.qll
@@ -62,7 +62,8 @@ class Assertion extends Comment {
     i = getPathLength() and
     result = API::root()
     or
-    result = lookup(i + 1).getASuccessor(getEdgeLabel(i))
+    result =
+      lookup(i + 1).getASuccessor(any(API::ApiLabel label | label.toString() = getEdgeLabel(i)))
   }
 
   predicate isNegative() { polarity = "!" }
@@ -79,7 +80,11 @@ class Assertion extends Comment {
       then
         suffix =
           "it does have outgoing edges labelled " +
-            concat(string lbl | exists(nd.getASuccessor(lbl)) | lbl, ", ") + "."
+            concat(string lbl |
+              exists(nd.getASuccessor(any(API::ApiLabel label | label.toString() = lbl)))
+            |
+              lbl, ", "
+            ) + "."
       else suffix = "it has no outgoing edges at all."
     |
       result = prefix + " " + suffix

From f39872e649486ee9ea029a7cf27050adb5d4ae03 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 19 Nov 2021 11:15:34 +0100
Subject: [PATCH 2/7] cache more predicates

---
 .../ql/lib/semmle/javascript/Constants.qll    | 28 ++++++++++++++++++-
 javascript/ql/lib/semmle/javascript/Expr.qll  |  9 ++++--
 .../javascript/MembershipCandidates.qll       |  1 +
 .../javascript/dataflow/TaintTracking.qll     |  4 ++-
 .../javascript/internal/CachedStages.qll      | 22 +++++++++++++++
 5 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/Constants.qll b/javascript/ql/lib/semmle/javascript/Constants.qll
index b9d47fbd8e8..d914dea8d9c 100644
--- a/javascript/ql/lib/semmle/javascript/Constants.qll
+++ b/javascript/ql/lib/semmle/javascript/Constants.qll
@@ -3,10 +3,12 @@
  */
 
 import javascript
+private import semmle.javascript.internal.CachedStages
 
 /**
  * An expression that evaluates to a constant primitive value.
  */
+cached
 abstract class ConstantExpr extends Expr { }
 
 /**
@@ -16,6 +18,7 @@ module SyntacticConstants {
   /**
    * An expression that evaluates to a constant value according to a bottom-up syntactic analysis.
    */
+  cached
   abstract class SyntacticConstant extends ConstantExpr { }
 
   /**
@@ -23,8 +26,11 @@ module SyntacticConstants {
    *
    * Note that `undefined`, `NaN` and `Infinity` are global variables, and are not covered by this class.
    */
+  cached
   class PrimitiveLiteralConstant extends SyntacticConstant {
+    cached
     PrimitiveLiteralConstant() {
+      Stages::Ast::ref() and
       this instanceof NumberLiteral
       or
       this instanceof StringLiteral
@@ -43,19 +49,27 @@ module SyntacticConstants {
   /**
    * A literal null expression.
    */
-  class NullConstant extends SyntacticConstant, NullLiteral { }
+  cached
+  class NullConstant extends SyntacticConstant, NullLiteral {
+    cached
+    NullConstant() { Stages::Ast::ref() and this = this }
+  }
 
   /**
    * A unary operation on a syntactic constant.
    */
+  cached
   class UnaryConstant extends SyntacticConstant, UnaryExpr {
+    cached
     UnaryConstant() { getOperand() instanceof SyntacticConstant }
   }
 
   /**
    * A binary operation on syntactic constants.
    */
+  cached
   class BinaryConstant extends SyntacticConstant, BinaryExpr {
+    cached
     BinaryConstant() {
       getLeftOperand() instanceof SyntacticConstant and
       getRightOperand() instanceof SyntacticConstant
@@ -65,7 +79,9 @@ module SyntacticConstants {
   /**
    * A conditional expression on syntactic constants.
    */
+  cached
   class ConditionalConstant extends SyntacticConstant, ConditionalExpr {
+    cached
     ConditionalConstant() {
       getCondition() instanceof SyntacticConstant and
       getConsequent() instanceof SyntacticConstant and
@@ -76,7 +92,9 @@ module SyntacticConstants {
   /**
    * A use of the global variable `undefined` or `void e`.
    */
+  cached
   class UndefinedConstant extends SyntacticConstant {
+    cached
     UndefinedConstant() {
       this.(GlobalVarAccess).getName() = "undefined" or
       this instanceof VoidExpr
@@ -86,21 +104,27 @@ module SyntacticConstants {
   /**
    * A use of the global variable `NaN`.
    */
+  cached
   class NaNConstant extends SyntacticConstant {
+    cached
     NaNConstant() { this.(GlobalVarAccess).getName() = "NaN" }
   }
 
   /**
    * A use of the global variable `Infinity`.
    */
+  cached
   class InfinityConstant extends SyntacticConstant {
+    cached
     InfinityConstant() { this.(GlobalVarAccess).getName() = "Infinity" }
   }
 
   /**
    * An expression that wraps the syntactic constant it evaluates to.
    */
+  cached
   class WrappedConstant extends SyntacticConstant {
+    cached
     WrappedConstant() { getUnderlyingValue() instanceof SyntacticConstant }
   }
 
@@ -123,6 +147,8 @@ module SyntacticConstants {
 /**
  * An expression that evaluates to a constant string.
  */
+cached
 class ConstantString extends ConstantExpr {
+  cached
   ConstantString() { exists(getStringValue()) }
 }
diff --git a/javascript/ql/lib/semmle/javascript/Expr.qll b/javascript/ql/lib/semmle/javascript/Expr.qll
index 4d91fdb218e..a9b881c6e53 100644
--- a/javascript/ql/lib/semmle/javascript/Expr.qll
+++ b/javascript/ql/lib/semmle/javascript/Expr.qll
@@ -89,7 +89,8 @@ class ExprOrType extends @expr_or_type, Documentable {
    *
    * Also see `getUnderlyingReference` and `stripParens`.
    */
-  Expr getUnderlyingValue() { result = this }
+  cached
+  Expr getUnderlyingValue() { Stages::Ast::ref() and result = this }
 }
 
 /**
@@ -274,7 +275,11 @@ private DataFlow::Node getCatchParameterFromStmt(Stmt stmt) {
  */
 class Identifier extends @identifier, ExprOrType {
   /** Gets the name of this identifier. */
-  string getName() { literals(result, _, this) }
+  cached
+  string getName() {
+    Stages::Ast::ref() and
+    literals(result, _, this)
+  }
 
   override string getAPrimaryQlClass() { result = "Identifier" }
 }
diff --git a/javascript/ql/lib/semmle/javascript/MembershipCandidates.qll b/javascript/ql/lib/semmle/javascript/MembershipCandidates.qll
index fe46eff040e..6c51b487f43 100644
--- a/javascript/ql/lib/semmle/javascript/MembershipCandidates.qll
+++ b/javascript/ql/lib/semmle/javascript/MembershipCandidates.qll
@@ -165,6 +165,7 @@ module MembershipCandidate {
     EnumerationRegExp enumeration;
     boolean polarity;
 
+    pragma[nomagic]
     RegExpEnumerationCandidate() {
       exists(DataFlow::MethodCallNode mcn, DataFlow::Node base, string m, DataFlow::Node firstArg |
         (
diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll
index 3b58b7e046f..cb3cdec8132 100644
--- a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll
+++ b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll
@@ -160,10 +160,12 @@ module TaintTracking {
    * of the standard library. Override `Configuration::isSanitizerGuard`
    * for analysis-specific taint sanitizer guards.
    */
+  cached
   abstract class AdditionalSanitizerGuardNode extends SanitizerGuardNode {
     /**
      * Holds if this guard applies to the flow in `cfg`.
      */
+    cached
     abstract predicate appliesTo(Configuration cfg);
   }
 
@@ -1127,7 +1129,7 @@ module TaintTracking {
         idx = astNode.getAnOperand() and
         idx.getPropertyNameExpr() = x and
         // and the other one is guaranteed to be `undefined`
-        forex(InferredType tp | tp = undef.getAType() | tp = TTUndefined())
+        unique(InferredType tp | tp = pragma[only_bind_into](undef.getAType())) = TTUndefined()
       )
     }
 
diff --git a/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll b/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
index da7658fe872..cda0727f136 100644
--- a/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
+++ b/javascript/ql/lib/semmle/javascript/internal/CachedStages.qll
@@ -69,6 +69,14 @@ module Stages {
       exists(any(Expr e).getStringValue())
       or
       any(ASTNode node).isAmbient()
+      or
+      exists(any(Identifier e).getName())
+      or
+      exists(any(ExprOrType e).getUnderlyingValue())
+      or
+      exists(ConstantExpr e)
+      or
+      exists(SyntacticConstants::NullConstant n)
     }
   }
 
@@ -299,6 +307,20 @@ module Stages {
       exists(Exports::getALibraryInputParameter())
       or
       any(RegExpTerm t).isUsedAsRegExp()
+      or
+      any(TaintTracking::AdditionalSanitizerGuardNode e).appliesTo(_)
+    }
+
+    cached
+    class DummySanitizer extends TaintTracking::AdditionalSanitizerGuardNode {
+      cached
+      DummySanitizer() { none() }
+
+      cached
+      override predicate appliesTo(TaintTracking::Configuration cfg) { none() }
+
+      cached
+      override predicate sanitizes(boolean outcome, Expr e) { none() }
     }
   }
 }

From c369b28a2a76cdef1c716e2fdb87680eafd7dab2 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 19 Nov 2021 11:15:59 +0100
Subject: [PATCH 3/7] optimizations in global data flow

---
 .../javascript/dataflow/Configuration.qll     | 66 +++++++++++--------
 1 file changed, 39 insertions(+), 27 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll
index de96295ce21..835c2f7e626 100644
--- a/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll
+++ b/javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll
@@ -939,18 +939,21 @@ private predicate basicFlowStepNoBarrier(
  * This predicate is field insensitive (it does not distinguish between `x` and `x.p`)
  * and hence should only be used for purposes of approximation.
  */
-pragma[inline]
+pragma[noinline]
 private predicate exploratoryFlowStep(
   DataFlow::Node pred, DataFlow::Node succ, DataFlow::Configuration cfg
 ) {
-  basicFlowStepNoBarrier(pred, succ, _, cfg) or
-  exploratoryLoadStep(pred, succ, cfg) or
-  isAdditionalLoadStoreStep(pred, succ, _, _, cfg) or
-  // the following three disjuncts taken together over-approximate flow through
-  // higher-order calls
-  exploratoryCallbackStep(pred, succ) or
-  succ = pred.(DataFlow::FunctionNode).getAParameter() or
-  exploratoryBoundInvokeStep(pred, succ)
+  isRelevantForward(pred, cfg) and
+  (
+    basicFlowStepNoBarrier(pred, succ, _, cfg) or
+    exploratoryLoadStep(pred, succ, cfg) or
+    isAdditionalLoadStoreStep(pred, succ, _, _, cfg) or
+    // the following three disjuncts taken together over-approximate flow through
+    // higher-order calls
+    exploratoryCallbackStep(pred, succ) or
+    succ = pred.(DataFlow::FunctionNode).getAParameter() or
+    exploratoryBoundInvokeStep(pred, succ)
+  )
 }
 
 /**
@@ -1024,6 +1027,7 @@ private string getAPropertyUsedInLoadStore(DataFlow::Configuration cfg) {
  * Holds if there exists a store-step from `pred` to `succ` under configuration `cfg`,
  * and somewhere in the program there exists a load-step that could possibly read the stored value.
  */
+pragma[noinline]
 private predicate exploratoryForwardStoreStep(
   DataFlow::Node pred, DataFlow::Node succ, DataFlow::Configuration cfg
 ) {
@@ -1075,8 +1079,10 @@ private string getABackwardsRelevantStoreProperty(DataFlow::Configuration cfg) {
 private predicate isRelevantForward(DataFlow::Node nd, DataFlow::Configuration cfg) {
   isSource(nd, cfg, _) and isLive()
   or
-  exists(DataFlow::Node mid | isRelevantForward(mid, cfg) |
-    exploratoryFlowStep(mid, nd, cfg) or
+  exists(DataFlow::Node mid |
+    exploratoryFlowStep(mid, nd, cfg)
+    or
+    isRelevantForward(mid, cfg) and
     exploratoryForwardStoreStep(mid, nd, cfg)
   )
 }
@@ -1098,11 +1104,10 @@ private predicate isRelevant(DataFlow::Node nd, DataFlow::Configuration cfg) {
 private predicate isRelevantBackStep(
   DataFlow::Node mid, DataFlow::Node nd, DataFlow::Configuration cfg
 ) {
+  exploratoryFlowStep(nd, mid, cfg)
+  or
   isRelevantForward(nd, cfg) and
-  (
-    exploratoryFlowStep(nd, mid, cfg) or
-    exploratoryBackwardStoreStep(nd, mid, cfg)
-  )
+  exploratoryBackwardStoreStep(nd, mid, cfg)
 }
 
 /**
@@ -1273,23 +1278,30 @@ private predicate parameterPropRead(
   DataFlow::Node arg, string prop, DataFlow::Node succ, DataFlow::Configuration cfg,
   PathSummary summary
 ) {
-  exists(Function f, DataFlow::Node read, DataFlow::Node invk |
+  exists(Function f, DataFlow::Node read, DataFlow::Node invk, DataFlow::Node parm |
+    reachesReturn(f, read, cfg, summary) and
+    parameterPropReadStep(parm, read, prop, cfg, arg, invk, f, succ)
+  )
+}
+
+// all the non-recursive parts of parameterPropRead outlined into a precomputed predicate
+pragma[noinline]
+private predicate parameterPropReadStep(
+  DataFlow::SourceNode parm, DataFlow::Node read, string prop, DataFlow::Configuration cfg,
+  DataFlow::Node arg, DataFlow::Node invk, Function f, DataFlow::Node succ
+) {
+  (
     not f.isAsyncOrGenerator() and invk = succ
     or
     // load from an immediately awaited function call
     f.isAsync() and
     invk = getAwaitOperand(succ)
-  |
-    exists(DataFlow::SourceNode parm |
-      callInputStep(f, invk, arg, parm, cfg) and
-      (
-        reachesReturn(f, read, cfg, summary) and
-        read = parm.getAPropertyRead(prop)
-        or
-        reachesReturn(f, read, cfg, summary) and
-        exists(DataFlow::Node use | parm.flowsTo(use) | isAdditionalLoadStep(use, read, prop, cfg))
-      )
-    )
+  ) and
+  callInputStep(f, invk, arg, parm, cfg) and
+  (
+    read = parm.getAPropertyRead(prop)
+    or
+    exists(DataFlow::Node use | parm.flowsTo(use) | isAdditionalLoadStep(use, read, prop, cfg))
   )
 }
 

From 6060f2e3e3c3eef2b53ad436db02d290f42c8555 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Fri, 19 Nov 2021 23:04:34 +0100
Subject: [PATCH 4/7] remove unused alias edge

---
 javascript/ql/lib/semmle/javascript/ApiGraphs.qll | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
index 740ac525b54..c90f542070c 100644
--- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
+++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
@@ -317,7 +317,6 @@ module API {
       or
       exists(Node pred, ApiLabel lbl, string predpath |
         Impl::edge(pred, lbl, this) and
-        not lbl instanceof Label::LabelAlias and
         predpath = pred.getAPath(length - 1) and
         exists(string space | if length = 1 then space = "" else space = " " |
           result = "(" + lbl + space + predpath + ")" and
@@ -1033,7 +1032,6 @@ module API {
       MkLabelReturn() or
       MkLabelPromised() or
       MkLabelPromisedError() or
-      MkLabelAlias() or
       MkLabelEntryPoint(API::EntryPoint e)
 
     class LabelEntryPoint extends ApiLabel {
@@ -1046,12 +1044,6 @@ module API {
       override string toString() { result = e }
     }
 
-    class LabelAlias extends ApiLabel {
-      LabelAlias() { this = MkLabelAlias() }
-
-      override string toString() { result = "" }
-    }
-
     class LabelPromised extends ApiLabel {
       LabelPromised() { this = MkLabelPromised() }
 
@@ -1174,9 +1166,6 @@ module API {
     /** Gets the `return` edge label. */
     LabelReturn return() { any() }
 
-    /** Gets the `alias` (empty) edge label. */
-    LabelAlias alias() { any() }
-
     /** Gets the `promised` edge label connecting a promise to its contained value. */
     MkLabelPromised promised() { any() }
 

From e9df860431f2b932bd04f1cc1e19494dd0e1963e Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Mon, 22 Nov 2021 09:43:13 +0100
Subject: [PATCH 5/7] refactor implementation to make Label implementations
 private

---
 .../ql/lib/semmle/javascript/ApiGraphs.qll    | 262 ++++++++++--------
 .../security/dataflow/RemoteFlowSources.qll   |  11 +-
 .../ql/src/meta/ApiGraphs/ApiGraphEdges.ql    |   2 +-
 .../ql/test/ApiGraphs/VerifyAssertions.qll    |   5 +-
 4 files changed, 151 insertions(+), 129 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
index c90f542070c..4aa3d20ffb7 100644
--- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
+++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
@@ -243,13 +243,13 @@ module API {
      * Gets a node such that there is an edge in the API graph between this node and the other
      * one, and that edge is labeled with `lbl`.
      */
-    Node getASuccessor(ApiLabel lbl) { Impl::edge(this, lbl, result) }
+    Node getASuccessor(Label::ApiLabel lbl) { Impl::edge(this, lbl, result) }
 
     /**
      * Gets a node such that there is an edge in the API graph between that other node and
      * this one, and that edge is labeled with `lbl`
      */
-    Node getAPredecessor(ApiLabel lbl) { this = result.getASuccessor(lbl) }
+    Node getAPredecessor(Label::ApiLabel lbl) { this = result.getASuccessor(lbl) }
 
     /**
      * Gets a node such that there is an edge in the API graph between this node and the other
@@ -315,7 +315,7 @@ module API {
       length = 0 and
       result = ""
       or
-      exists(Node pred, ApiLabel lbl, string predpath |
+      exists(Node pred, Label::ApiLabel lbl, string predpath |
         Impl::edge(pred, lbl, this) and
         predpath = pred.getAPath(length - 1) and
         exists(string space | if length = 1 then space = "" else space = " " |
@@ -383,9 +383,7 @@ module API {
     abstract DataFlow::Node getARhs();
 
     /** Gets an API-node for this entry point. */
-    API::Node getNode() {
-      result = root().getASuccessor(any(Label::LabelEntryPoint l | l.getEntryPoint() = this))
-    }
+    API::Node getNode() { result = root().getASuccessor(Label::entryPoint(this)) }
   }
 
   /**
@@ -474,11 +472,14 @@ module API {
      * incoming edge from `base` labeled `lbl` in the API graph.
      */
     cached
-    predicate rhs(TApiNode base, ApiLabel lbl, DataFlow::Node rhs) {
+    predicate rhs(TApiNode base, Label::ApiLabel lbl, DataFlow::Node rhs) {
       hasSemantics(rhs) and
       (
         base = MkRoot() and
-        rhs = lbl.(Label::LabelEntryPoint).getEntryPoint().getARhs()
+        exists(EntryPoint e |
+          lbl = Label::entryPoint(e) and
+          rhs = e.getARhs()
+        )
         or
         exists(string m, string prop |
           base = MkModuleExport(m) and
@@ -590,7 +591,7 @@ module API {
      */
     pragma[noinline]
     private predicate propertyRead(
-      DataFlow::SourceNode pred, string propDesc, ApiLabel lbl, DataFlow::Node ref
+      DataFlow::SourceNode pred, string propDesc, Label::ApiLabel lbl, DataFlow::Node ref
     ) {
       ref = pred.getAPropertyRead() and
       lbl = Label::memberFromRef(ref) and
@@ -614,11 +615,14 @@ module API {
      * `lbl` in the API graph.
      */
     cached
-    predicate use(TApiNode base, ApiLabel lbl, DataFlow::Node ref) {
+    predicate use(TApiNode base, Label::ApiLabel lbl, DataFlow::Node ref) {
       hasSemantics(ref) and
       (
         base = MkRoot() and
-        ref = lbl.(Label::LabelEntryPoint).getEntryPoint().getAUse()
+        exists(EntryPoint e |
+          lbl = Label::entryPoint(e) and
+          ref = e.getAUse()
+        )
         or
         // property reads
         exists(DataFlow::SourceNode src, DataFlow::SourceNode pred, string propDesc |
@@ -863,7 +867,7 @@ module API {
      * Holds if there is an edge from `pred` to `succ` in the API graph that is labeled with `lbl`.
      */
     cached
-    predicate edge(TApiNode pred, ApiLabel lbl, TApiNode succ) {
+    predicate edge(TApiNode pred, Label::ApiLabel lbl, TApiNode succ) {
       Stages::APIStage::ref() and
       exists(string m |
         pred = MkRoot() and
@@ -941,8 +945,6 @@ module API {
     }
   }
 
-  import Label as EdgeLabel
-
   /**
    * An `InvokeNode` that is connected to the API graph.
    *
@@ -999,109 +1001,12 @@ module API {
   /** A `new` call connected to the API graph. */
   class NewNode extends InvokeNode, DataFlow::NewNode { }
 
-  /** A label in the API-graph */
-  abstract class ApiLabel extends Label::TLabel {
-    string toString() { result = "???" }
-  }
-
-  private module Label {
-    newtype TLabel =
-      MkLabelMod(string mod) {
-        exists(Impl::MkModuleExport(mod)) or
-        exists(Impl::MkModuleImport(mod))
-      } or
-      MkLabelInstance() or
-      MkLabelMember(string prop) {
-        exports(_, prop, _) or
-        exists(any(DataFlow::ClassNode c).getInstanceMethod(prop)) or
-        prop = "exports" or
-        prop = any(CanonicalName c).getName() or
-        prop = any(DataFlow::PropRef p).getPropertyName() or
-        exists(Impl::MkTypeUse(_, prop)) or
-        exists(any(Module m).getAnExportedValue(prop))
-      } or
-      MkLabelUnknownMember() or
-      MkLabelParameter(int i) {
-        i =
-          [-1 .. max(int args |
-              args = any(InvokeExpr invk).getNumArgument() or
-              args = any(Function f).getNumParameter()
-            )] or
-        i = [0 .. 10]
-      } or
-      MkLabelReturn() or
-      MkLabelPromised() or
-      MkLabelPromisedError() or
-      MkLabelEntryPoint(API::EntryPoint e)
-
-    class LabelEntryPoint extends ApiLabel {
-      API::EntryPoint e;
-
-      LabelEntryPoint() { this = MkLabelEntryPoint(e) }
-
-      API::EntryPoint getEntryPoint() { result = e }
-
-      override string toString() { result = e }
-    }
-
-    class LabelPromised extends ApiLabel {
-      LabelPromised() { this = MkLabelPromised() }
-
-      override string toString() { result = "promised" }
-    }
-
-    class LabelPromisedError extends ApiLabel {
-      LabelPromisedError() { this = MkLabelPromisedError() }
-
-      override string toString() { result = "promised" }
-    }
-
-    class LabelReturn extends ApiLabel {
-      LabelReturn() { this = MkLabelReturn() }
-
-      override string toString() { result = "return" }
-    }
-
-    class LabelMod extends ApiLabel {
-      string mod;
-
-      LabelMod() { this = MkLabelMod(mod) }
-
-      string getMod() { result = mod }
-
-      override string toString() { result = "module " + mod }
-    }
-
-    class LabelInstance extends ApiLabel {
-      LabelInstance() { this = MkLabelInstance() }
-
-      override string toString() { result = "instance" }
-    }
-
-    class LabelMember extends ApiLabel {
-      string prop;
-
-      LabelMember() { this = MkLabelMember(prop) }
-
-      string getProperty() { result = prop }
-
-      override string toString() { result = "member " + prop }
-    }
-
-    class LabelUnknownMember extends ApiLabel {
-      LabelUnknownMember() { this = MkLabelUnknownMember() }
-
-      override string toString() { result = "member *" }
-    }
-
-    class LabelParameter extends ApiLabel {
-      int i;
-
-      LabelParameter() { this = MkLabelParameter(i) }
-
-      override string toString() { result = "parameter " + i }
-
-      int getIndex() { result = i }
+  /** Provides classes modeling the various edges (labels) in the API graph. */
+  module Label {
+    /** A label in the API-graph */
+    abstract class ApiLabel extends TLabel {
+      /** Gets a string representation of this label. */
+      string toString() { result = "???" }
     }
 
     /** Gets the edge label for the module `m`. */
@@ -1167,10 +1072,129 @@ module API {
     LabelReturn return() { any() }
 
     /** Gets the `promised` edge label connecting a promise to its contained value. */
-    MkLabelPromised promised() { any() }
+    LabelPromised promised() { any() }
 
     /** Gets the `promisedError` edge label connecting a promise to its rejected value. */
-    MkLabelPromisedError promisedError() { any() }
+    LabelPromisedError promisedError() { any() }
+
+    /** Gets an entry-point label for the entry-point `e`. */
+    LabelEntryPoint entryPoint(API::EntryPoint e) { result.getEntryPoint() = e }
+
+    private import LabelImpl
+
+    private module LabelImpl {
+      newtype TLabel =
+        MkLabelMod(string mod) {
+          exists(Impl::MkModuleExport(mod)) or
+          exists(Impl::MkModuleImport(mod))
+        } or
+        MkLabelInstance() or
+        MkLabelMember(string prop) {
+          exports(_, prop, _) or
+          exists(any(DataFlow::ClassNode c).getInstanceMethod(prop)) or
+          prop = "exports" or
+          prop = any(CanonicalName c).getName() or
+          prop = any(DataFlow::PropRef p).getPropertyName() or
+          exists(Impl::MkTypeUse(_, prop)) or
+          exists(any(Module m).getAnExportedValue(prop))
+        } or
+        MkLabelUnknownMember() or
+        MkLabelParameter(int i) {
+          i =
+            [-1 .. max(int args |
+                args = any(InvokeExpr invk).getNumArgument() or
+                args = any(Function f).getNumParameter()
+              )] or
+          i = [0 .. 10]
+        } or
+        MkLabelReturn() or
+        MkLabelPromised() or
+        MkLabelPromisedError() or
+        MkLabelEntryPoint(API::EntryPoint e)
+
+      /** A label for an entry-point. */
+      class LabelEntryPoint extends ApiLabel {
+        API::EntryPoint e;
+
+        LabelEntryPoint() { this = MkLabelEntryPoint(e) }
+
+        /** Gets the EntryPoint associated with this label. */
+        API::EntryPoint getEntryPoint() { result = e }
+
+        override string toString() { result = e }
+      }
+
+      /** A label that gets a promised value. */
+      class LabelPromised extends ApiLabel {
+        LabelPromised() { this = MkLabelPromised() }
+
+        override string toString() { result = "promised" }
+      }
+
+      /** A label that gets a rejected promise. */
+      class LabelPromisedError extends ApiLabel {
+        LabelPromisedError() { this = MkLabelPromisedError() }
+
+        override string toString() { result = "promisedError" }
+      }
+
+      /** A label that gets the return value of a function. */
+      class LabelReturn extends ApiLabel {
+        LabelReturn() { this = MkLabelReturn() }
+
+        override string toString() { result = "return" }
+      }
+
+      /** A label for a module. */
+      class LabelMod extends ApiLabel {
+        string mod;
+
+        LabelMod() { this = MkLabelMod(mod) }
+
+        /** Gets the module associated with this label. */
+        string getMod() { result = mod }
+
+        override string toString() { result = "module " + mod }
+      }
+
+      /** A label that gets an instance from a `new` call. */
+      class LabelInstance extends ApiLabel {
+        LabelInstance() { this = MkLabelInstance() }
+
+        override string toString() { result = "instance" }
+      }
+
+      /** A label for the member named `prop`. */
+      class LabelMember extends ApiLabel {
+        string prop;
+
+        LabelMember() { this = MkLabelMember(prop) }
+
+        /** Gets the property associated with this label. */
+        string getProperty() { result = prop }
+
+        override string toString() { result = "member " + prop }
+      }
+
+      /** A label for a member with an unknown name. */
+      class LabelUnknownMember extends ApiLabel {
+        LabelUnknownMember() { this = MkLabelUnknownMember() }
+
+        override string toString() { result = "member *" }
+      }
+
+      /** A label for parameter `i`. */
+      class LabelParameter extends ApiLabel {
+        int i;
+
+        LabelParameter() { this = MkLabelParameter(i) }
+
+        override string toString() { result = "parameter " + i }
+
+        /** Gets the index of the parameter for this label. */
+        int getIndex() { result = i }
+      }
+    }
   }
 }
 
diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll
index 81fb8dd2cf5..2aba7f8fed6 100644
--- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll
+++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll
@@ -117,17 +117,14 @@ private class RemoteFlowSourceAccessPath extends JSONString {
   string getSourceType() { result = sourceType }
 
   /** Gets the `i`th component of the access path specifying this remote flow source. */
-  API::ApiLabel getComponent(int i) {
+  API::Label::ApiLabel getComponent(int i) {
     exists(string raw | raw = this.getValue().splitAt(".", i + 1) |
       i = 0 and
-      result
-          .(API::EdgeLabel::LabelEntryPoint)
-          .getEntryPoint()
-          .(ExternalRemoteFlowSourceSpecEntryPoint)
-          .getName() = raw
+      result =
+        API::Label::entryPoint(any(ExternalRemoteFlowSourceSpecEntryPoint e | e.getName() = raw))
       or
       i > 0 and
-      result = API::EdgeLabel::member(raw)
+      result = API::Label::member(raw)
     )
   }
 
diff --git a/javascript/ql/src/meta/ApiGraphs/ApiGraphEdges.ql b/javascript/ql/src/meta/ApiGraphs/ApiGraphEdges.ql
index 1920bfb183f..23bdad59754 100644
--- a/javascript/ql/src/meta/ApiGraphs/ApiGraphEdges.ql
+++ b/javascript/ql/src/meta/ApiGraphs/ApiGraphEdges.ql
@@ -12,4 +12,4 @@ import javascript
 import meta.MetaMetrics
 
 select projectRoot(),
-  count(API::Node pred, API::ApiLabel lbl, API::Node succ | succ = pred.getASuccessor(lbl))
+  count(API::Node pred, API::Label::ApiLabel lbl, API::Node succ | succ = pred.getASuccessor(lbl))
diff --git a/javascript/ql/test/ApiGraphs/VerifyAssertions.qll b/javascript/ql/test/ApiGraphs/VerifyAssertions.qll
index 6a57d85518a..1c8c494863e 100644
--- a/javascript/ql/test/ApiGraphs/VerifyAssertions.qll
+++ b/javascript/ql/test/ApiGraphs/VerifyAssertions.qll
@@ -63,7 +63,8 @@ class Assertion extends Comment {
     result = API::root()
     or
     result =
-      lookup(i + 1).getASuccessor(any(API::ApiLabel label | label.toString() = getEdgeLabel(i)))
+      lookup(i + 1)
+          .getASuccessor(any(API::Label::ApiLabel label | label.toString() = getEdgeLabel(i)))
   }
 
   predicate isNegative() { polarity = "!" }
@@ -81,7 +82,7 @@ class Assertion extends Comment {
         suffix =
           "it does have outgoing edges labelled " +
             concat(string lbl |
-              exists(nd.getASuccessor(any(API::ApiLabel label | label.toString() = lbl)))
+              exists(nd.getASuccessor(any(API::Label::ApiLabel label | label.toString() = lbl)))
             |
               lbl, ", "
             ) + "."

From 148da611c6ca20604be2945bd9b7e45f01631142 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Wed, 1 Dec 2021 13:45:52 +0100
Subject: [PATCH 6/7] make the ApiLabel class non-abstract

---
 javascript/ql/lib/semmle/javascript/ApiGraphs.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
index 2188eb8fadc..e51c5a82fdc 100644
--- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
+++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
@@ -1006,7 +1006,7 @@ module API {
   /** Provides classes modeling the various edges (labels) in the API graph. */
   module Label {
     /** A label in the API-graph */
-    abstract class ApiLabel extends TLabel {
+    class ApiLabel extends TLabel {
       /** Gets a string representation of this label. */
       string toString() { result = "???" }
     }

From 0a3d62c92a956677e791629761a17f9198855760 Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Wed, 1 Dec 2021 13:48:16 +0100
Subject: [PATCH 7/7] rename `mod` -> `module`

---
 javascript/ql/lib/semmle/javascript/ApiGraphs.qll | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
index e51c5a82fdc..0f8dd3178d7 100644
--- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
+++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll
@@ -873,7 +873,7 @@ module API {
       Stages::APIStage::ref() and
       exists(string m |
         pred = MkRoot() and
-        lbl = Label::mod(m)
+        lbl = Label::moduleLabel(m)
       |
         succ = MkModuleDef(m)
         or
@@ -1012,7 +1012,7 @@ module API {
     }
 
     /** Gets the edge label for the module `m`. */
-    LabelMod mod(string m) { result.getMod() = m }
+    LabelModule moduleLabel(string m) { result.getMod() = m }
 
     /** Gets the `member` edge label for member `m`. */
     bindingset[m]
@@ -1086,7 +1086,7 @@ module API {
 
     private module LabelImpl {
       newtype TLabel =
-        MkLabelMod(string mod) {
+        MkLabelModule(string mod) {
           exists(Impl::MkModuleExport(mod)) or
           exists(Impl::MkModuleImport(mod))
         } or
@@ -1148,10 +1148,10 @@ module API {
       }
 
       /** A label for a module. */
-      class LabelMod extends ApiLabel {
+      class LabelModule extends ApiLabel {
         string mod;
 
-        LabelMod() { this = MkLabelMod(mod) }
+        LabelModule() { this = MkLabelModule(mod) }
 
         /** Gets the module associated with this label. */
         string getMod() { result = mod }