JS: Update with new AdditionalTaintStep subclasses

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)
     }
   }
 }

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

@@ -7,45 +7,45 @@ import javascript
 /**
  * A taint step for the `marked` library, that converts markdown to HTML.
  */
-private class MarkedStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  MarkedStep() {
-    this = DataFlow::globalVarRef("marked").getACall()
-    or
-    this = DataFlow::moduleImport("marked").getACall()
-  }
-
+private class MarkedStep extends TaintTracking::SharedTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    succ = this and
-    pred = this.getArgument(0)
+    exists(DataFlow::CallNode call |
+      call = DataFlow::globalVarRef("marked").getACall()
+      or
+      call = DataFlow::moduleImport("marked").getACall()
+    |
+      succ = call and
+      pred = call.getArgument(0)
+    )
   }
 }
 
 /**
  * A taint step for the `markdown-table` library.
  */
-private class MarkdownTableStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  MarkdownTableStep() { this = DataFlow::moduleImport("markdown-table").getACall() }
-
+private class MarkdownTableStep extends TaintTracking::SharedTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    succ = this and
-    pred = this.getArgument(0)
+    exists(DataFlow::CallNode call | call = DataFlow::moduleImport("markdown-table").getACall() |
+      succ = call and
+      pred = call.getArgument(0)
+    )
   }
 }
 
 /**
  * A taint step for the `showdown` library.
  */
-private class ShowDownStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  ShowDownStep() {
-    this =
-      [DataFlow::globalVarRef("showdown"), DataFlow::moduleImport("showdown")]
-          .getAConstructorInvocation("Converter")
-          .getAMemberCall(["makeHtml", "makeMd"])
-  }
-
+private class ShowDownStep extends TaintTracking::SharedTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    succ = this and
-    pred = this.getArgument(0)
+    exists(DataFlow::CallNode call |
+      call =
+        [DataFlow::globalVarRef("showdown"), DataFlow::moduleImport("showdown")]
+            .getAConstructorInvocation("Converter")
+            .getAMemberCall(["makeHtml", "makeMd"])
+    |
+      succ = call and
+      pred = call.getArgument(0)
+    )
   }
 }
 
@@ -93,16 +93,16 @@ private module Unified {
   /**
    * A taint step for the `unified` library.
    */
-  class UnifiedStep extends TaintTracking::AdditionalTaintStep, UnifiedChain {
-    UnifiedStep() {
-      // sanitizer. Mostly looking for `rehype-sanitize`, but also other plugins with `sanitize` in their name.
-      not this.getAUsedPlugin().getALocalSource() =
-        DataFlow::moduleImport(any(string s | s.matches("%sanitize%")))
-    }
-
+  class UnifiedStep extends TaintTracking::SharedTaintStep {
     override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      pred = getInput() and
-      succ = getOutput()
+      exists(UnifiedChain chain |
+        // sanitizer. Mostly looking for `rehype-sanitize`, but also other plugins with `sanitize` in their name.
+        not chain.getAUsedPlugin().getALocalSource() =
+          DataFlow::moduleImport(any(string s | s.matches("%sanitize%")))
+      |
+        pred = chain.getInput() and
+        succ = chain.getOutput()
+      )
     }
   }
 }
@@ -110,11 +110,11 @@ private module Unified {
 /**
  * A taint step for the `snarkdown` library.
  */
-private class SnarkdownStep extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
-  SnarkdownStep() { this = DataFlow::moduleImport("snarkdown").getACall() }
-
+private class SnarkdownStep extends TaintTracking::SharedTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    this = succ and
-    pred = this.getArgument(0)
+    exists(DataFlow::CallNode call | call = DataFlow::moduleImport("snarkdown").getACall() |
+      call = succ and
+      pred = call.getArgument(0)
+    )
   }
 }

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

@@ -7,12 +7,12 @@ private import javascript
 /**
  * A step `x -> y` in `x.subscribe(y => ...)`, modeling flow out of an rxjs Observable.
  */
-private class RxJsSubscribeStep extends TaintTracking::AdditionalTaintStep, DataFlow::MethodCallNode {
-  RxJsSubscribeStep() { getMethodName() = "subscribe" }
-
-  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = getReceiver() and
-    succ = getCallback(0).getParameter(0)
+private class RxJsSubscribeStep extends TaintTracking::SharedTaintStep {
+  override predicate heapStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(DataFlow::MethodCallNode call | call.getMethodName() = "subscribe" |
+      pred = call.getReceiver() and
+      succ = call.getCallback(0).getParameter(0)
+    )
   }
 }
 
@@ -54,24 +54,24 @@ private predicate isIdentityPipe(DataFlow::CallNode pipe) {
 /**
  * A step in or out of the map callback in a call of form `x.pipe(map(y => ...))`.
  */
-private class RxJsPipeMapStep extends TaintTracking::AdditionalTaintStep, DataFlow::MethodCallNode {
-  RxJsPipeMapStep() { getMethodName() = "pipe" }
-
-  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    pred = getReceiver() and
-    succ = pipeInput(getArgument(0).getALocalSource())
-    or
-    exists(int i |
-      pred = pipeOutput(getArgument(i).getALocalSource()) and
-      succ = pipeInput(getArgument(i + 1).getALocalSource())
+private class RxJsPipeMapStep extends TaintTracking::SharedTaintStep {
+  override predicate heapStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(DataFlow::MethodCallNode call | call.getMethodName() = "pipe" |
+      pred = call.getReceiver() and
+      succ = pipeInput(call.getArgument(0).getALocalSource())
+      or
+      exists(int i |
+        pred = pipeOutput(call.getArgument(i).getALocalSource()) and
+        succ = pipeInput(call.getArgument(i + 1).getALocalSource())
+      )
+      or
+      pred = pipeOutput(call.getLastArgument().getALocalSource()) and
+      succ = call
+      or
+      // Handle a common case where the last step is `catchError`.
+      isIdentityPipe(call.getLastArgument().getALocalSource()) and
+      pred = pipeOutput(call.getArgument(call.getNumArgument() - 2)) and
+      succ = call
     )
-    or
-    pred = pipeOutput(getLastArgument().getALocalSource()) and
-    succ = this
-    or
-    // Handle a common case where the last step is `catchError`.
-    isIdentityPipe(getLastArgument().getALocalSource()) and
-    pred = pipeOutput(getArgument(getNumArgument() - 2)) and
-    succ = this
   }
 }

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

@@ -276,14 +276,12 @@ module XML {
     }
   }
 
-  private class XMLParserTaintStep extends js::TaintTracking::AdditionalTaintStep {
-    XML::ParserInvocation parser;
-
-    XMLParserTaintStep() { this.asExpr() = parser }
-
-    override predicate step(js::DataFlow::Node pred, js::DataFlow::Node succ) {
-      pred.asExpr() = parser.getSourceArgument() and
-      succ = parser.getAResult()
+  private class XMLParserTaintStep extends js::TaintTracking::SharedTaintStep {
+    override predicate deserializeStep(js::DataFlow::Node pred, js::DataFlow::Node succ) {
+      exists(XML::ParserInvocation parser |
+        pred.asExpr() = parser.getSourceArgument() and
+        succ = parser.getAResult()
+      )
     }
   }
 }

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

@@ -71,11 +71,9 @@ module DomBasedXss {
       DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl,
       DataFlow::FlowLabel succlbl
     ) {
-      exists(TaintTracking::AdditionalTaintStep step |
-        step.step(pred, succ) and
-        predlbl.isData() and
-        succlbl.isTaint()
-      )
+      TaintTracking::sharedTaintStep(pred, succ) and
+      predlbl.isData() and
+      succlbl.isTaint()
     }
 
     override predicate isSanitizer(DataFlow::Node node) {

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

@@ -255,7 +255,7 @@ module ExternalAPIUsedWithUntrustedData {
       not exists(DataFlow::Node arg |
         arg = this.getAnArgument() and not arg instanceof DeepObjectSink
       |
-        any(TaintTracking::AdditionalTaintStep s).step(arg, _)
+        TaintTracking::sharedTaintStep(arg, _)
         or
         exists(DataFlow::AdditionalFlowStep s |
           s.step(arg, _) or