Merge pull request #5499 from asgerf/js/non-recursive-sourcenode

Approved by erik-krogh

Author: codeql-ci

javascript/ql/src/semmle/javascript/ApiGraphs.qll

@@ -619,11 +619,11 @@ module API {
     cached
     predicate use(TApiNode nd, DataFlow::Node ref) {
       exists(string m, Module mod | nd = MkModuleDef(m) and mod = importableModule(m) |
-        ref.(ModuleAsSourceNode).getModule() = mod
+        ref.(ModuleVarNode).getModule() = mod
       )
       or
       exists(string m, Module mod | nd = MkModuleExport(m) and mod = importableModule(m) |
-        ref.(ExportsAsSourceNode).getModule() = mod
+        ref.(ExportsVarNode).getModule() = mod
         or
         exists(DataFlow::Node base | use(MkModuleDef(m), base) |
           ref = trackUseNode(base).getAPropertyRead("exports")
@@ -746,9 +746,9 @@ module API {
       or
       // additional backwards step from `require('m')` to `exports` or `module.exports` in m
       exists(Import imp | imp.getImportedModuleNode() = trackDefNode(nd, t.continue()) |
-        result.(ExportsAsSourceNode).getModule() = imp.getImportedModule()
+        result.(ExportsVarNode).getModule() = imp.getImportedModule()
         or
-        exists(ModuleAsSourceNode mod |
+        exists(ModuleVarNode mod |
           mod.getModule() = imp.getImportedModule() and
           result = mod.(DataFlow::SourceNode).getAPropertyRead("exports")
         )
@@ -983,31 +983,44 @@ private module Label {
   string promised() { result = "promised" }
 }
 
+private class NodeModuleSourcesNodes extends DataFlow::SourceNode::Range {
+  Variable v;
+
+  NodeModuleSourcesNodes() {
+    exists(NodeModule m |
+      this = DataFlow::ssaDefinitionNode(SSA::implicitInit(v)) and
+      v = [m.getModuleVariable(), m.getExportsVariable()]
+    )
+  }
+
+  Variable getVariable() { result = v }
+}
+
 /**
- * A CommonJS/AMD `module` variable, considered as a source node.
+ * A CommonJS/AMD `module` variable.
  */
-private class ModuleAsSourceNode extends DataFlow::SourceNode::Range {
+private class ModuleVarNode extends DataFlow::Node {
   Module m;
 
-  ModuleAsSourceNode() {
-    this = DataFlow::ssaDefinitionNode(SSA::implicitInit(m.(NodeModule).getModuleVariable()))
+  ModuleVarNode() {
+    this.(NodeModuleSourcesNodes).getVariable() = m.(NodeModule).getModuleVariable()
     or
-    this = DataFlow::parameterNode(m.(AmdModule).getDefine().getModuleParameter())
+    DataFlow::parameterNode(this, m.(AmdModule).getDefine().getModuleParameter())
   }
 
   Module getModule() { result = m }
 }
 
 /**
- * A CommonJS/AMD `exports` variable, considered as a source node.
+ * A CommonJS/AMD `exports` variable.
  */
-private class ExportsAsSourceNode extends DataFlow::SourceNode::Range {
+private class ExportsVarNode extends DataFlow::Node {
   Module m;
 
-  ExportsAsSourceNode() {
-    this = DataFlow::ssaDefinitionNode(SSA::implicitInit(m.(NodeModule).getExportsVariable()))
+  ExportsVarNode() {
+    this.(NodeModuleSourcesNodes).getVariable() = m.(NodeModule).getExportsVariable()
     or
-    this = DataFlow::parameterNode(m.(AmdModule).getDefine().getExportsParameter())
+    DataFlow::parameterNode(this, m.(AmdModule).getDefine().getExportsParameter())
   }
 
   Module getModule() { result = m }

javascript/ql/src/semmle/javascript/DOM.qll

@@ -315,6 +315,11 @@ module DOM {
       )
     }
 
+    private InferredType getArgumentTypeFromJQueryMethodGet(JQuery::MethodCall call) {
+      call.getMethodName() = "get" and
+      result = call.getArgument(0).analyze().getAType()
+    }
+
     private class DefaultRange extends Range {
       DefaultRange() {
         this.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable
@@ -344,7 +349,7 @@ module DOM {
         or
         exists(JQuery::MethodCall call | this = call and call.getMethodName() = "get" |
           call.getNumArgument() = 1 and
-          unique(InferredType t | t = call.getArgument(0).analyze().getAType()) = TTNumber()
+          unique(InferredType t | t = getArgumentTypeFromJQueryMethodGet(call)) = TTNumber()
         )
         or
         // A `this` node from a callback given to a `$().each(callback)` call.

javascript/ql/src/semmle/javascript/HtmlSanitizers.qll

@@ -16,51 +16,53 @@ abstract class HtmlSanitizerCall extends DataFlow::CallNode {
   abstract DataFlow::Node getInput();
 }
 
+pragma[noinline]
+private DataFlow::SourceNode htmlSanitizerFunction() {
+  result = DataFlow::moduleMember("ent", "encode")
+  or
+  result = DataFlow::moduleMember("entities", "encodeHTML")
+  or
+  result = DataFlow::moduleMember("entities", "encodeXML")
+  or
+  result = DataFlow::moduleMember("escape-goat", "escape")
+  or
+  result = DataFlow::moduleMember("he", "encode")
+  or
+  result = DataFlow::moduleMember("he", "escape")
+  or
+  result = DataFlow::moduleImport("sanitize-html")
+  or
+  result = DataFlow::moduleMember("sanitizer", "escape")
+  or
+  result = DataFlow::moduleMember("sanitizer", "sanitize")
+  or
+  result = DataFlow::moduleMember("validator", "escape")
+  or
+  result = DataFlow::moduleImport("xss")
+  or
+  result = DataFlow::moduleMember("xss-filters", _)
+  or
+  result = LodashUnderscore::member("escape")
+  or
+  exists(DataFlow::PropRead read | read = result |
+    read.getPropertyName() = "sanitize" and
+    read.getBase().asExpr().(VarAccess).getName() = "DOMPurify"
+  )
+  or
+  exists(string name | name = "encode" or name = "encodeNonUTF" |
+    result = DataFlow::moduleMember("html-entities", _).getAnInstantiation().getAPropertyRead(name) or
+    result = DataFlow::moduleMember("html-entities", _).getAPropertyRead(name)
+  )
+  or
+  result = Closure::moduleImport("goog.string.htmlEscape")
+}
+
 /**
  * Matches HTML sanitizers from known NPM packages as well as home-made sanitizers (matched by name).
  */
 private class DefaultHtmlSanitizerCall extends HtmlSanitizerCall {
   DefaultHtmlSanitizerCall() {
-    exists(DataFlow::SourceNode callee | this = callee.getACall() |
-      callee = DataFlow::moduleMember("ent", "encode")
-      or
-      callee = DataFlow::moduleMember("entities", "encodeHTML")
-      or
-      callee = DataFlow::moduleMember("entities", "encodeXML")
-      or
-      callee = DataFlow::moduleMember("escape-goat", "escape")
-      or
-      callee = DataFlow::moduleMember("he", "encode")
-      or
-      callee = DataFlow::moduleMember("he", "escape")
-      or
-      callee = DataFlow::moduleImport("sanitize-html")
-      or
-      callee = DataFlow::moduleMember("sanitizer", "escape")
-      or
-      callee = DataFlow::moduleMember("sanitizer", "sanitize")
-      or
-      callee = DataFlow::moduleMember("validator", "escape")
-      or
-      callee = DataFlow::moduleImport("xss")
-      or
-      callee = DataFlow::moduleMember("xss-filters", _)
-      or
-      callee = LodashUnderscore::member("escape")
-      or
-      exists(DataFlow::PropRead read | read = callee |
-        read.getPropertyName() = "sanitize" and
-        read.getBase().asExpr().(VarAccess).getName() = "DOMPurify"
-      )
-      or
-      exists(string name | name = "encode" or name = "encodeNonUTF" |
-        callee =
-          DataFlow::moduleMember("html-entities", _).getAnInstantiation().getAPropertyRead(name) or
-        callee = DataFlow::moduleMember("html-entities", _).getAPropertyRead(name)
-      )
-      or
-      callee = Closure::moduleImport("goog.string.htmlEscape")
-    )
+    this = htmlSanitizerFunction().getACall()
     or
     // Match home-made sanitizers by name.
     exists(string calleeName | calleeName = getCalleeName() |

javascript/ql/src/semmle/javascript/Promises.qll

@@ -596,6 +596,7 @@ private module ClosurePromise {
    * A promise created by a call `goog.Promise.resolve(value)`.
    */
   private class ResolvedClosurePromiseDefinition extends ResolvedPromiseDefinition {
+    pragma[noinline]
     ResolvedClosurePromiseDefinition() {
       this = Closure::moduleImport("goog.Promise.resolve").getACall()
     }

javascript/ql/src/semmle/javascript/StringConcatenation.qll

@@ -55,12 +55,8 @@ module StringConcatenation {
     exists(DataFlow::MethodCallNode call |
       node = call and
       call.getMethodName() = "concat" and
-      not (
-        exists(DataFlow::ArrayCreationNode array |
-          array.flowsTo(call.getAnArgument()) or array.flowsTo(call.getReceiver())
-        )
-        or
-        DataFlow::reflectiveCallNode(_) = call
+      not exists(DataFlow::ArrayCreationNode array |
+        array.flowsTo(call.getAnArgument()) or array.flowsTo(call.getReceiver())
       ) and
       (
         n = 0 and

javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

@@ -59,18 +59,16 @@ class ParameterNode extends DataFlow::SourceNode {
  * ```
  */
 class InvokeNode extends DataFlow::SourceNode {
-  DataFlow::Impl::InvokeNodeDef impl;
-
-  InvokeNode() { this = impl }
+  InvokeNode() { this instanceof DataFlow::Impl::InvokeNodeDef }
 
   /** Gets the syntactic invoke expression underlying this function invocation. */
-  InvokeExpr getInvokeExpr() { result = impl.getInvokeExpr() }
+  InvokeExpr getInvokeExpr() { result = this.(DataFlow::Impl::InvokeNodeDef).getInvokeExpr() }
 
   /** Gets the name of the function or method being invoked, if it can be determined. */
-  string getCalleeName() { result = impl.getCalleeName() }
+  string getCalleeName() { result = this.(DataFlow::Impl::InvokeNodeDef).getCalleeName() }
 
   /** Gets the data flow node specifying the function to be called. */
-  DataFlow::Node getCalleeNode() { result = impl.getCalleeNode() }
+  DataFlow::Node getCalleeNode() { result = this.(DataFlow::Impl::InvokeNodeDef).getCalleeNode() }
 
   /**
    * Gets the data flow node corresponding to the `i`th argument of this invocation.
@@ -91,10 +89,10 @@ class InvokeNode extends DataFlow::SourceNode {
    * but the position of `z` cannot be determined, hence there are no first and second
    * argument nodes.
    */
-  DataFlow::Node getArgument(int i) { result = impl.getArgument(i) }
+  DataFlow::Node getArgument(int i) { result = this.(DataFlow::Impl::InvokeNodeDef).getArgument(i) }
 
   /** Gets the data flow node corresponding to an argument of this invocation. */
-  DataFlow::Node getAnArgument() { result = impl.getAnArgument() }
+  DataFlow::Node getAnArgument() { result = this.(DataFlow::Impl::InvokeNodeDef).getAnArgument() }
 
   /** Gets the data flow node corresponding to the last argument of this invocation. */
   DataFlow::Node getLastArgument() { result = getArgument(getNumArgument() - 1) }
@@ -111,10 +109,12 @@ class InvokeNode extends DataFlow::SourceNode {
    * ```
    *  .
    */
-  DataFlow::Node getASpreadArgument() { result = impl.getASpreadArgument() }
+  DataFlow::Node getASpreadArgument() {
+    result = this.(DataFlow::Impl::InvokeNodeDef).getASpreadArgument()
+  }
 
   /** Gets the number of arguments of this invocation, if it can be determined. */
-  int getNumArgument() { result = impl.getNumArgument() }
+  int getNumArgument() { result = this.(DataFlow::Impl::InvokeNodeDef).getNumArgument() }
 
   Function getEnclosingFunction() { result = getBasicBlock().getContainer() }
 
@@ -256,14 +256,14 @@ class InvokeNode extends DataFlow::SourceNode {
  * ```
  */
 class CallNode extends InvokeNode {
-  override DataFlow::Impl::CallNodeDef impl;
+  CallNode() { this instanceof DataFlow::Impl::CallNodeDef }
 
   /**
    * Gets the data flow node corresponding to the receiver expression of this method call.
    *
    * For example, the receiver of `x.m()` is `x`.
    */
-  DataFlow::Node getReceiver() { result = impl.getReceiver() }
+  DataFlow::Node getReceiver() { result = this.(DataFlow::Impl::CallNodeDef).getReceiver() }
 }
 
 /**
@@ -277,10 +277,10 @@ class CallNode extends InvokeNode {
  * ```
  */
 class MethodCallNode extends CallNode {
-  override DataFlow::Impl::MethodCallNodeDef impl;
+  MethodCallNode() { this instanceof DataFlow::Impl::MethodCallNodeDef }
 
   /** Gets the name of the invoked method, if it can be determined. */
-  string getMethodName() { result = impl.getMethodName() }
+  string getMethodName() { result = this.(DataFlow::Impl::MethodCallNodeDef).getMethodName() }
 
   /**
    * Holds if this data flow node calls method `methodName` on receiver node `receiver`.
@@ -300,7 +300,7 @@ class MethodCallNode extends CallNode {
  * ```
  */
 class NewNode extends InvokeNode {
-  override DataFlow::Impl::NewNodeDef impl;
+  NewNode() { this instanceof DataFlow::Impl::NewNodeDef }
 }
 
 /**

javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll

@@ -268,9 +268,11 @@ module SocketIO {
 
     /** Gets the `i`th parameter through which data is received from a client. */
     override DataFlow::SourceNode getReceivedItem(int i) {
-      exists(DataFlow::FunctionNode cb | cb = getListener() and result = cb.getParameter(i) |
+      exists(DataFlow::FunctionNode cb |
+        cb = getListener() and
+        result = cb.getParameter(i) and
         // exclude last parameter if it looks like a callback
-        result != cb.getLastParameter() or not exists(result.getAnInvocation())
+        not (result = cb.getLastParameter() and exists(result.getAnInvocation()))
       )
     }
 

javascript/ql/src/semmle/javascript/security/dataflow/DOM.qll

@@ -156,6 +156,12 @@ private module PersistentWebStorage {
     result = DataFlow::globalVarRef(kind)
   }
 
+  pragma[noinline]
+  WriteAccess getAWriteByName(string name, string kind) {
+    result.getKey() = name and
+    result.getKind() = kind
+  }
+
   /**
    * A read access.
    */
@@ -165,8 +171,10 @@ private module PersistentWebStorage {
     ReadAccess() { this = webStorage(kind).getAMethodCall("getItem") }
 
     override PersistentWriteAccess getAWrite() {
-      getArgument(0).mayHaveStringValue(result.(WriteAccess).getKey()) and
-      result.(WriteAccess).getKind() = kind
+      exists(string name |
+        getArgument(0).mayHaveStringValue(name) and
+        result = getAWriteByName(name, kind)
+      )
     }
   }