JS: Update with new AdditionalTaintStep subclasses

Author: asgerf

javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql

@@ -27,7 +27,7 @@ private DataFlow::Node remoteFlow(DataFlow::TypeTracker t) {
   exists(DataFlow::TypeTracker t2, DataFlow::Node prev | prev = remoteFlow(t2) |
     t2 = t.smallstep(prev, result)
     or
-    any(TaintTracking::AdditionalTaintStep dts).step(prev, result) and
+    TaintTracking::sharedTaintStep(prev, result) and
     t = t2
   )
 }

javascript/ql/src/experimental/semmle/javascript/security/dataflow/ResourceExhaustion.qll

@@ -37,7 +37,7 @@ module ResourceExhaustion {
   }
 
   predicate isRestrictedAdditionalTaintStep(DataFlow::Node src, DataFlow::Node dst) {
-    any(TaintTracking::AdditionalTaintStep dts).step(src, dst) and
+    TaintTracking::sharedTaintStep(src, dst) and
     not dst.asExpr() instanceof AddExpr and
     not dst.(DataFlow::MethodCallNode).calls(src, "toString")
   }

javascript/ql/src/meta/analysis-quality/TaintSteps.ql

@@ -13,7 +13,7 @@ import CallGraphQuality
 
 predicate relevantStep(DataFlow::Node pred, DataFlow::Node succ) {
   (
-    any(TaintTracking::AdditionalTaintStep dts).step(pred, succ)
+    TaintTracking::sharedTaintStep(pred, succ)
     or
     any(DataFlow::AdditionalFlowStep cfg).step(pred, succ)
     or

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

@@ -491,14 +491,13 @@ private module AsyncReturnSteps {
   /**
    * A data-flow step for ordinary return from an async function in a taint configuration.
    */
-  private class AsyncTaintReturn extends TaintTracking::AdditionalTaintStep, DataFlow::FunctionNode {
-    Function f;
-
-    AsyncTaintReturn() { this.getFunction() = f and f.isAsync() }
-
+  private class AsyncTaintReturn extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      returnExpr(f, pred, _) and
-      succ.(DataFlow::FunctionReturnNode).getFunction() = f
+      exists(Function f |
+        f.isAsync() and
+        returnExpr(f, pred, _) and
+        succ.(DataFlow::FunctionReturnNode).getFunction() = f
+      )
     }
   }
 }

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

@@ -180,9 +180,7 @@ module Angular2 {
     )
   }
 
-  private class AngularTaintStep extends TaintTracking::AdditionalTaintStep {
-    AngularTaintStep() { taintStep(_, this) }
-
+  private class AngularTaintStep extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) { taintStep(pred, succ) }
   }
 
@@ -483,14 +481,12 @@ module Angular2 {
    * A taint step `array -> elem` in `*ngFor="let elem of array"`, or more precisely,
    * a step from `array` to each access to `elem`.
    */
-  private class ForLoopStep extends TaintTracking::AdditionalTaintStep {
-    ForLoopAttribute attrib;
-
-    ForLoopStep() { this = attrib.getIterationDomain() }
-
+  private class ForLoopStep extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this and
-      succ = attrib.getAnIteratorAccess()
+      exists(ForLoopAttribute attrib |
+        pred = attrib.getIterationDomain() and
+        succ = attrib.getAnIteratorAccess()
+      )
     }
   }
 
@@ -513,27 +509,26 @@ module Angular2 {
     result.getCalleeNode().asExpr().(PipeRefExpr).getName() = name
   }
 
-  private class BuiltinPipeStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    string name;
-
-    BuiltinPipeStep() { this = getAPipeCall(name) }
-
+  private class BuiltinPipeStep extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      succ = this and
-      exists(int i | pred = getArgument(i) |
-        i = 0 and
-        name =
-          [
-            "async", "i18nPlural", "json", "keyvalue", "lowercase", "uppercase", "titlecase",
-            "slice"
-          ]
+      exists(DataFlow::CallNode call, string name |
+        call = getAPipeCall(name) and
+        succ = call
+      |
+        exists(int i | pred = call.getArgument(i) |
+          i = 0 and
+          name =
+            [
+              "async", "i18nPlural", "json", "keyvalue", "lowercase", "uppercase", "titlecase",
+              "slice"
+            ]
+          or
+          i = 1 and name = "date" // date format string
+        )
         or
-        i = 1 and name = "date" // date format string
+        name = "translate" and
+        pred = [call.getArgument(1), call.getOptionArgument(1, _)]
       )
-      or
-      name = "translate" and
-      succ = this and
-      pred = [getArgument(1), getOptionArgument(1, _)]
     }
   }
 
@@ -582,27 +577,23 @@ module Angular2 {
    * </mat-table>
    * ```
    */
-  private class MatTableTaintStep extends TaintTracking::AdditionalTaintStep {
-    MatTableElement table;
-
-    MatTableTaintStep() { this = table.getDataSourceNode() }
-
+  private class MatTableTaintStep extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this and
-      succ = table.getARowRef()
+      exists(MatTableElement table |
+        pred = table.getDataSourceNode() and
+        succ = table.getARowRef()
+      )
     }
   }
 
   /** A taint step into the data array of a `MatTableDataSource` instance. */
-  private class MatTableDataSourceStep extends TaintTracking::AdditionalTaintStep, DataFlow::NewNode {
-    MatTableDataSourceStep() {
-      this =
-        DataFlow::moduleMember("@angular/material/table", "MatTableDataSource").getAnInstantiation()
-    }
-
+  private class MatTableDataSourceStep extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = [getArgument(0), getAPropertyWrite("data").getRhs()] and
-      succ = this
+      exists(DataFlow::NewNode invoke |
+        invoke = DataFlow::moduleMember("@angular/material/table", "MatTableDataSource").getAnInstantiation() and
+        pred = [invoke.getArgument(0), invoke.getAPropertyWrite("data").getRhs()] and
+        succ = invoke
+      )
     }
   }
 }

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

@@ -8,32 +8,24 @@ private DataFlow::SourceNode classnames() {
   result = DataFlow::moduleImport(["classnames", "classnames/dedupe", "classnames/bind"])
 }
 
-private class PlainStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  PlainStep() {
-    this = classnames().getACall()
-    or
-    this = DataFlow::moduleImport("clsx").getACall()
-  }
-
+private class PlainStep extends TaintTracking::SharedTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = getAnArgument() and
-    succ = this
+    exists(DataFlow::CallNode call |
+      call = [classnames().getACall(), DataFlow::moduleImport("clsx").getACall()] and
+      pred = call.getAnArgument() and
+      succ = call
+    )
   }
 }
 
 /**
  * Step from `x` or `y` to the result of `classnames.bind(x)(y)`.
  */
-private class BindStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  DataFlow::CallNode bind;
-
-  BindStep() {
-    bind = classnames().getAMemberCall("bind") and
-    this = bind.getACall()
-  }
-
+private class BindStep extends TaintTracking::SharedTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = [getAnArgument(), bind.getAnArgument(), bind.getOptionArgument(_, _)] and
-    succ = this
+    exists(DataFlow::CallNode bind | bind = classnames().getAMemberCall("bind") |
+      pred = [succ.(DataFlow::CallNode).getAnArgument(), bind.getAnArgument(), bind.getOptionArgument(_, _)] and
+      succ = bind.getACall()
+    )
   }
 }

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

@@ -29,24 +29,26 @@ private module DateFns {
    *
    * A format string can use single-quotes to include mostly arbitrary text.
    */
-  private class FormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    FormatStep() { this = formatFunction().getACall() }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(1) and
-      succ = this
+  private class FormatStep extends TaintTracking::SharedTaintStep {
+    override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call |
+        call = formatFunction().getACall() and
+        pred = call.getArgument(1) and
+        succ = call
+      )
     }
   }
 
   /**
    * Taint step of form: `f -> format(f)(date)`
    */
-  private class CurriedFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    CurriedFormatStep() { this = curriedFormatFunction().getACall() }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(0) and
-      succ = getACall()
+  private class CurriedFormatStep extends TaintTracking::SharedTaintStep {
+    override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call |
+        call = curriedFormatFunction().getACall() and
+        pred = call.getArgument(0) and
+        succ = call.getACall()
+      )
     }
   }
 }
@@ -66,12 +68,13 @@ private module Moment {
    *
    * The format string can use backslash-escaping to include mostly arbitrary text.
    */
-  private class MomentFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    MomentFormatStep() { this = moment().getMember("format").getACall() }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(0) and
-      succ = this
+  private class MomentFormatStep extends TaintTracking::SharedTaintStep {
+    override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call |
+        call = moment().getMember("format").getACall() and
+        pred = call.getArgument(0) and
+        succ = call
+      )
     }
   }
 }
@@ -82,12 +85,13 @@ private module DateFormat {
    *
    * The format string can use single-quotes to include mostly arbitrary text.
    */
-  private class DateFormatStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    DateFormatStep() { this = DataFlow::moduleImport("dateformat").getACall() }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getArgument(1) and
-      succ = this
+  private class DateFormatStep extends TaintTracking::SharedTaintStep {
+    override predicate stringManipulationStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call |
+        call = DataFlow::moduleImport("dateformat").getACall() and
+        pred = call.getArgument(1) and
+        succ = call
+      )
     }
   }
 }

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

@@ -11,12 +11,13 @@ private module JwtDecode {
   /**
    * A taint-step for `succ = require("jwt-decode")(pred)`.
    */
-  private class JwtDecodeStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    JwtDecodeStep() { this = DataFlow::moduleImport("jwt-decode").getACall() }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this.getArgument(0) and
-      succ = this
+  private class JwtDecodeStep extends TaintTracking::SharedTaintStep {
+    override predicate deserializeStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call |
+        call = DataFlow::moduleImport("jwt-decode").getACall() and
+        pred = call.getArgument(0) and
+        succ = call
+      )
     }
   }
 }
@@ -28,12 +29,13 @@ private module JsonWebToken {
   /**
    * A taint-step for `require("jsonwebtoken").verify(pred, "key", (err succ) => {...})`.
    */
-  private class VerifyStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-    VerifyStep() { this = DataFlow::moduleMember("jsonwebtoken", "verify").getACall() }
-
+  private class VerifyStep extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = this.getArgument(0) and
-      succ = this.getABoundCallbackParameter(2, 1)
+      exists(DataFlow::CallNode call |
+        call = DataFlow::moduleMember("jsonwebtoken", "verify").getACall() and
+        pred = call.getArgument(0) and
+        succ = call.getABoundCallbackParameter(2, 1)
+      )
     }
   }
 

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

@@ -440,11 +440,9 @@ module LodashUnderscore {
   /**
    * A model for taint-steps involving (non-function) underscore methods.
    */
-  private class UnderscoreTaintStep extends TaintTracking::AdditionalTaintStep {
-    UnderscoreTaintStep() { underscoreTaintStep(this, _) }
-
+  private class UnderscoreTaintStep extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      underscoreTaintStep(pred, succ) and pred = this
+      underscoreTaintStep(pred, succ)
     }
   }
 }