add database accesses as additional (heuristic) remote flow sources

Author: Stephan Brandauer

javascript/ql/lib/semmle/javascript/Concepts.qll

@@ -84,6 +84,21 @@ abstract class FileNameSource extends DataFlow::Node { }
 abstract class DatabaseAccess extends DataFlow::Node {
   /** Gets an argument to this database access that is interpreted as a query. */
   abstract DataFlow::Node getAQueryArgument();
+
+  /** Gets a node to which a result of the access may flow. */
+  DataFlow::Node getAResult() {
+    none() // Overridden in subclass
+  }
+
+  /**
+   * Holds if the data returned can be a user-controlled object,
+   * such as a JSON object parsed from user-controlled data.
+   */
+  predicate returnsUserControlledObject() {
+    // NB: Most data bases support JSON data (some via plugins),
+    // which is why this has a default implementation.
+    any()
+  }
 }
 
 /**

javascript/ql/lib/semmle/javascript/frameworks/Knex.qll

@@ -46,17 +46,35 @@ module Knex {
     RawKnexSqlString() { this = any(RawKnexCall call).getArgument(0).asExpr() }
   }
 
-  /** A call that triggers a SQL query submission. */
-  private class KnexDatabaseAccess extends DatabaseAccess {
-    KnexDatabaseAccess() {
-      this = knexObject().getMember(["then", "stream", "asCallback"]).getACall()
+  /** A call that triggers a SQL query submission by calling then/stream/asCallback. */
+  private class KnexDatabaseCallback extends DatabaseAccess, DataFlow::CallNode {
+    string member;
+
+    KnexDatabaseCallback() {
+      member = ["then", "stream", "asCallback"] and
+      this = knexObject().getMember(member).getACall()
+    }
+
+    override DataFlow::Node getAResult() {
+      member = "then" and
+      result = this.getCallback(0).getParameter(0)
       or
-      exists(AwaitExpr await |
-        this = await.flow() and
-        await.getOperand() = knexObject().getAUse().asExpr()
+      member = "asCallback" and
+      result = this.getCallback(0).getParameter(1)
+    }
+
+    override DataFlow::Node getAQueryArgument() { none() }
+  }
+
+  private class KnexDatabaseAwait extends DatabaseAccess, DataFlow::ValueNode {
+    KnexDatabaseAwait() {
+      exists(AwaitExpr enclosingAwait | this = enclosingAwait.flow() |
+        enclosingAwait.getOperand() = knexObject().getAUse().asExpr()
       )
     }
 
+    override DataFlow::Node getAResult() { result = this }
+
     override DataFlow::Node getAQueryArgument() { none() }
   }
 }

javascript/ql/lib/semmle/javascript/frameworks/NoSQL.qll

@@ -3,6 +3,7 @@
  */
 
 import javascript
+import semmle.javascript.Promises
 
 module NoSQL {
   /** An expression that is interpreted as a NoSQL query. */
@@ -65,6 +66,10 @@ private module MongoDB {
 
     override DataFlow::Node getAQueryArgument() { result = this.getArgument(queryArgIdx) }
 
+    override DataFlow::Node getAResult() {
+      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
+    }
+
     DataFlow::Node getACodeOperator() {
       result = getADollarWhereProperty(this.getParameter(queryArgIdx))
     }
@@ -537,12 +542,29 @@ private module Mongoose {
       // NB: the complete information is not easily accessible for deeply chained calls
       f.getQueryArgument().getARhs() = result
     }
+
+    override DataFlow::Node getAResult() {
+      result = this.getCallback(this.getNumArgument() - 1).getParameter(1)
+    }
   }
 
-  class ExplicitQueryEvaluation extends DatabaseAccess {
+  class ExplicitQueryEvaluation extends DatabaseAccess, DataFlow::CallNode {
+    string member;
+
     ExplicitQueryEvaluation() {
       // explicit execution using a Query method call
-      Query::getAMongooseQuery().getMember(["exec", "then", "catch"]).getACall() = this
+      member = ["exec", "then", "catch"] and
+      Query::getAMongooseQuery().getMember(member).getACall() = this
+    }
+
+    private int resultParamIndex() {
+      member = "then" and result = 0
+      or
+      member = "exec" and result = 1
+    }
+
+    override DataFlow::Node getAResult() {
+      result = this.getCallback(_).getParameter(this.resultParamIndex())
     }
 
     override DataFlow::Node getAQueryArgument() {
@@ -588,6 +610,10 @@ private module Minimongo {
 
     override DataFlow::Node getAQueryArgument() { result = this.getArgument(queryArgIdx) }
 
+    override DataFlow::Node getAResult() {
+      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
+    }
+
     DataFlow::Node getACodeOperator() {
       result = getADollarWhereProperty(this.getParameter(queryArgIdx))
     }
@@ -609,7 +635,7 @@ private module Minimongo {
  * Provides classes modeling the MarsDB library.
  */
 private module MarsDB {
-  private class MarsDBAccess extends DatabaseAccess {
+  private class MarsDBAccess extends DatabaseAccess, DataFlow::CallNode {
     string method;
 
     MarsDBAccess() {
@@ -623,21 +649,29 @@ private module MarsDB {
 
     string getMethod() { result = method }
 
+    override DataFlow::Node getAResult() {
+      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
+    }
+
     override DataFlow::Node getAQueryArgument() { none() }
   }
 
   /** A call to a MarsDB query method. */
-  private class QueryCall extends DatabaseAccess, API::CallNode {
+  private class QueryCall extends MarsDBAccess, API::CallNode {
     int queryArgIdx;
 
     QueryCall() {
       exists(string m |
-        this.(MarsDBAccess).getMethod() = m and
+        this.getMethod() = m and
         // implements parts of the Minimongo interface
         Minimongo::CollectionMethodSignatures::interpretsArgumentAsQuery(m, queryArgIdx)
       )
     }
 
+    override DataFlow::Node getAResult() {
+      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
+    }
+
     override DataFlow::Node getAQueryArgument() { result = this.getArgument(queryArgIdx) }
 
     DataFlow::Node getACodeOperator() {
@@ -744,9 +778,13 @@ private module Redis {
   /**
    * An access to a database through redis
    */
-  class RedisDatabaseAccess extends DatabaseAccess {
+  class RedisDatabaseAccess extends DatabaseAccess, DataFlow::CallNode {
     RedisDatabaseAccess() { this = redis().getMember(_).getACall() }
 
+    override DataFlow::Node getAResult() {
+      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
+    }
+
     override DataFlow::Node getAQueryArgument() { none() }
   }
 }
@@ -768,9 +806,13 @@ private module IoRedis {
   /**
    * An access to a database through ioredis
    */
-  class IoRedisDatabaseAccess extends DatabaseAccess {
+  class IoRedisDatabaseAccess extends DatabaseAccess, DataFlow::CallNode {
     IoRedisDatabaseAccess() { this = ioredis().getMember(_).getACall() }
 
+    override DataFlow::Node getAResult() {
+      PromiseFlow::loadStep(this.getALocalUse(), result, Promises::valueProp())
+    }
+
     override DataFlow::Node getAQueryArgument() { none() }
   }
 }