update the SQL/NoSQL models to use dataflow nodes

Author: erik-krogh

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/NosqlInjectionATM.qll

@@ -115,7 +115,7 @@ predicate isBaseAdditionalFlowStep(
   inlbl = TaintedObject::label() and
   outlbl = TaintedObject::label() and
   exists(NoSql::Query query, DataFlow::SourceNode queryObj |
-    queryObj.flowsToExpr(query) and
+    queryObj.flowsTo(query) and
     queryObj.flowsTo(trg) and
     src = queryObj.getAPropertyWrite().getRhs()
   )

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

@@ -43,7 +43,7 @@ module Knex {
 
   /** A SQL string passed to a raw Knex method. */
   private class RawKnexSqlString extends SQL::SqlString {
-    RawKnexSqlString() { this = any(RawKnexCall call).getArgument(0).asExpr() }
+    RawKnexSqlString() { this = any(RawKnexCall call).getArgument(0) }
   }
 
   /** A call that triggers a SQL query submission by calling then/stream/asCallback. */

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

@@ -6,8 +6,8 @@ import javascript
 
 /** Provides classes for modeling NoSql query sinks. */
 module NoSql {
-  /** An expression that is interpreted as a NoSql query. */
-  abstract class Query extends Expr {
+  /** An expression that is interpreted as a NoSQL query. */
+  abstract class Query extends DataFlow::Node {
     /** Gets an expression that is interpreted as a code operator in this query. */
     DataFlow::Node getACodeOperator() { none() }
   }
@@ -84,7 +84,7 @@ private module MongoDB {
   class Query extends NoSql::Query {
     QueryCall qc;
 
-    Query() { this = qc.getAQueryArgument().asExpr() }
+    Query() { this = qc.getAQueryArgument() }
 
     override DataFlow::Node getACodeOperator() { result = qc.getACodeOperator() }
   }
@@ -518,7 +518,7 @@ private module Mongoose {
   class MongoDBQueryPart extends NoSql::Query {
     MongooseFunction f;
 
-    MongoDBQueryPart() { this = f.getQueryArgument().asSink().asExpr() }
+    MongoDBQueryPart() { this = f.getQueryArgument().asSink() }
 
     override DataFlow::Node getACodeOperator() {
       result = getADollarWhereProperty(f.getQueryArgument())
@@ -625,7 +625,7 @@ private module Minimongo {
   class Query extends NoSql::Query {
     QueryCall qc;
 
-    Query() { this = qc.getAQueryArgument().asExpr() }
+    Query() { this = qc.getAQueryArgument() }
 
     override DataFlow::Node getACodeOperator() { result = qc.getACodeOperator() }
   }
@@ -685,7 +685,7 @@ private module MarsDB {
   class Query extends NoSql::Query {
     QueryCall qc;
 
-    Query() { this = qc.getAQueryArgument().asExpr() }
+    Query() { this = qc.getAQueryArgument() }
 
     override DataFlow::Node getACodeOperator() { result = qc.getACodeOperator() }
   }
@@ -770,7 +770,7 @@ private module Redis {
     RedisKeyArgument() {
       exists(string method, int argIndex |
         QuerySignatures::argumentIsAmbiguousKey(method, argIndex) and
-        this = redis().getMember(method).getParameter(argIndex).asSink().asExpr()
+        this = redis().getMember(method).getParameter(argIndex).asSink()
       )
     }
   }

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

@@ -5,26 +5,26 @@
 import javascript
 
 module SQL {
-  /** A string-valued expression that is interpreted as a SQL command. */
-  abstract class SqlString extends Expr { }
+  /** A string-valued dataflow node that is interpreted as a SQL command. */
+  abstract class SqlString extends DataFlow::Node { }
 
   private class SqlStringFromModel extends SqlString {
-    SqlStringFromModel() { this = ModelOutput::getASinkNode("sql-injection").asSink().asExpr() }
+    SqlStringFromModel() { this = ModelOutput::getASinkNode("sql-injection").asSink() }
   }
 
   /**
-   * An expression that sanitizes a string to make it safe to embed into
+   * An dataflow node that sanitizes a string to make it safe to embed into
    * a SQL command.
    */
-  abstract class SqlSanitizer extends Expr {
-    Expr input;
-    Expr output;
+  abstract class SqlSanitizer extends DataFlow::Node {
+    DataFlow::Node input;
+    DataFlow::Node output;
 
     /** Gets the input expression being sanitized. */
-    Expr getInput() { result = input }
+    DataFlow::Node getInput() { result = input }
 
     /** Gets the output expression containing the sanitized value. */
-    Expr getOutput() { result = output }
+    DataFlow::Node getOutput() { result = output }
   }
 }
 
@@ -90,13 +90,13 @@ private module MySql {
 
   /** An expression that is passed to the `query` method and hence interpreted as SQL. */
   class QueryString extends SQL::SqlString {
-    QueryString() { this = any(QueryCall qc).getAQueryArgument().asExpr() }
+    QueryString() { this = any(QueryCall qc).getAQueryArgument() }
   }
 
   /** A call to the `escape` or `escapeId` method that performs SQL sanitization. */
-  class EscapingSanitizer extends SQL::SqlSanitizer, MethodCallExpr {
+  class EscapingSanitizer extends SQL::SqlSanitizer instanceof API::CallNode {
     EscapingSanitizer() {
-      this = [mysql(), pool(), connection()].getMember(["escape", "escapeId"]).getACall().asExpr() and
+      this = [mysql(), pool(), connection()].getMember(["escape", "escapeId"]).getACall() and
       input = this.getArgument(0) and
       output = this
     }
@@ -198,9 +198,9 @@ private module Postgres {
   /** An expression that is passed to the `query` method and hence interpreted as SQL. */
   class QueryString extends SQL::SqlString {
     QueryString() {
-      this = any(QueryCall qc).getAQueryArgument().asExpr()
+      this = any(QueryCall qc).getAQueryArgument()
       or
-      this = API::moduleImport("pg-cursor").getParameter(0).asSink().asExpr()
+      this = API::moduleImport("pg-cursor").getParameter(0).asSink()
     }
   }
 
@@ -349,7 +349,7 @@ private module Postgres {
 
   /** An expression that is interpreted as SQL by `pg-promise`. */
   class PgPromiseQueryString extends SQL::SqlString {
-    PgPromiseQueryString() { this = any(PgPromiseQueryCall qc).getAQueryArgument().asExpr() }
+    PgPromiseQueryString() { this = any(PgPromiseQueryCall qc).getAQueryArgument() }
   }
 }
 
@@ -398,7 +398,7 @@ private module Sqlite {
 
   /** An expression that is passed to the `query` method and hence interpreted as SQL. */
   class QueryString extends SQL::SqlString {
-    QueryString() { this = any(QueryCall qc).getAQueryArgument().asExpr() }
+    QueryString() { this = any(QueryCall qc).getAQueryArgument() }
   }
 }
 
@@ -470,15 +470,15 @@ private module MsSql {
   class QueryString extends SQL::SqlString {
     QueryString() {
       exists(DatabaseAccess dba | dba instanceof QueryTemplateExpr or dba instanceof QueryCall |
-        this = dba.getAQueryArgument().asExpr()
+        this = dba.getAQueryArgument()
       )
     }
   }
 
   /** An element of a query template, which is automatically sanitized. */
   class QueryTemplateSanitizer extends SQL::SqlSanitizer {
     QueryTemplateSanitizer() {
-      this = any(QueryTemplateExpr qte).getAQueryArgument().asExpr() and
+      this = any(QueryTemplateExpr qte).getAQueryArgument() and
       input = this and
       output = this
     }

javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionCustomizations.qll

@@ -36,7 +36,5 @@ module NosqlInjection {
   }
 
   /** An expression interpreted as a NoSql query, viewed as a sink. */
-  class NosqlQuerySink extends Sink, DataFlow::ValueNode {
-    override NoSql::Query astNode;
-  }
+  class NosqlQuerySink extends Sink instanceof NoSql::Query { }
 }

javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionQuery.qll

@@ -45,7 +45,7 @@ class Configuration extends TaintTracking::Configuration {
     inlbl = TaintedObject::label() and
     outlbl = TaintedObject::label() and
     exists(NoSql::Query query, DataFlow::SourceNode queryObj |
-      queryObj.flowsToExpr(query) and
+      queryObj.flowsTo(query) and
       queryObj.flowsTo(trg) and
       src = queryObj.getAPropertyWrite().getRhs()
     )

javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionCustomizations.qll

@@ -28,13 +28,11 @@ module SqlInjection {
   }
 
   /** An SQL expression passed to an API call that executes SQL. */
-  class SqlInjectionExprSink extends Sink, DataFlow::ValueNode {
-    override SQL::SqlString astNode;
-  }
+  class SqlInjectionExprSink extends Sink instanceof SQL::SqlString { }
 
   /** An expression that sanitizes a value for the purposes of string based query injection. */
-  class SanitizerExpr extends Sanitizer, DataFlow::ValueNode {
-    SanitizerExpr() { astNode = any(SQL::SqlSanitizer ss).getOutput() }
+  class SanitizerExpr extends Sanitizer {
+    SanitizerExpr() { this = any(SQL::SqlSanitizer ss).getOutput() }
   }
 
   /** An GraphQL expression passed to an API call that executes GraphQL. */

javascript/ql/test/tutorials/Introducing the JavaScript libraries/query20.qll

@@ -1,5 +1,5 @@
 import javascript
 
 query predicate test_query20(SQL::SqlString ss, string res) {
-  ss instanceof AddExpr and res = "Use templating instead of string concatenation."
+  ss.asExpr() instanceof AddExpr and res = "Use templating instead of string concatenation."
 }