Merge pull request #1367 from xiemaisi/js/configuration-api-consistency

Approved by esben-semmle

Author: semmle-qlci

change-notes/1.21/analysis-javascript.md

@@ -50,6 +50,7 @@
 
 * `RegExpLiteral` is now a `DataFlow::SourceNode`.
 * `JSDocTypeExpr` now has source locations and is a subclass of `Locatable` and `TypeAnnotation`.
+* The two-parameter versions of predicate `isBarrier` in `DataFlow::Configuration` and of predicate `isSanitizer` in `TaintTracking::Configuration` have been renamed to `isBarrierEdge` and `isSanitizerEdge`, respectively. The old names are maintained for backwards-compatibility in this version, but will be deprecated in the next version and subsequently removed.
 * Various predicates named `getTypeAnnotation()` now return `TypeAnnotation` instead of `TypeExpr`.
   In rare cases, this may cause compilation errors in existing code. Cast the result to `TypeExpr` if this happens.
 * The `getALabel` predicate in `LabeledBarrierGuardNode` and `LabeledSanitizerGuardNode`

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

@@ -153,15 +153,29 @@ abstract class Configuration extends string {
   }
 
   /**
+   * DEPRECATED: Use `isBarrierEdge` instead.
+   *
    * Holds if flow from `src` to `trg` is prohibited.
    */
   predicate isBarrier(DataFlow::Node src, DataFlow::Node trg) { none() }
 
   /**
+   * DEPRECATED: Use `isBarrierEdge` instead.
+   *
    * Holds if flow with label `lbl` cannot flow from `src` to `trg`.
    */
   predicate isBarrier(DataFlow::Node src, DataFlow::Node trg, FlowLabel lbl) { none() }
 
+  /**
+   * Holds if flow from `pred` to `succ` is prohibited.
+   */
+  predicate isBarrierEdge(DataFlow::Node pred, DataFlow::Node succ) { none() }
+
+  /**
+   * Holds if flow with label `lbl` cannot flow from `pred` to `succ`.
+   */
+  predicate isBarrierEdge(DataFlow::Node pred, DataFlow::Node succ, FlowLabel lbl) { none() }
+
   /**
    * Holds if flow with label `lbl` cannot flow into `node`.
    */
@@ -473,6 +487,7 @@ private predicate basicFlowStep(
     exists(FlowLabel predlbl, FlowLabel succlbl |
       localFlowStep(pred, succ, cfg, predlbl, succlbl) and
       not cfg.isBarrier(pred, succ, predlbl) and
+      not cfg.isBarrierEdge(pred, succ, predlbl) and
       summary = MkPathSummary(false, false, predlbl, succlbl)
     )
     or
@@ -586,7 +601,8 @@ private predicate callInputStep(
     )
   ) and
   not cfg.isBarrier(succ) and
-  not cfg.isBarrier(pred, succ)
+  not cfg.isBarrier(pred, succ) and
+  not cfg.isBarrierEdge(pred, succ)
 }
 
 /**
@@ -641,6 +657,7 @@ private predicate flowThroughCall(
     calls(output, f) and // Do not consider partial calls
     reachableFromInput(f, output, input, ret, cfg, summary) and
     not cfg.isBarrier(ret, output) and
+    not cfg.isBarrierEdge(ret, output) and
     not cfg.isLabeledBarrier(output, summary.getEndLabel())
   )
   or
@@ -650,6 +667,7 @@ private predicate flowThroughCall(
     calls(invk, f) and
     reachableFromInput(f, invk, input, ret, cfg, summary) and
     not cfg.isBarrier(ret, output) and
+    not cfg.isBarrierEdge(ret, output) and
     not cfg.isLabeledBarrier(output, summary.getEndLabel())
   )
 }
@@ -836,6 +854,7 @@ private predicate flowStep(
   ) and
   not cfg.isBarrier(succ) and
   not cfg.isBarrier(pred, succ) and
+  not cfg.isBarrierEdge(pred, succ) and
   not cfg.isLabeledBarrier(succ, summary.getEndLabel())
 }
 

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

@@ -51,14 +51,30 @@ module TaintTracking {
     /** Holds if the intermediate node `node` is a taint sanitizer. */
     predicate isSanitizer(DataFlow::Node node) { none() }
 
-    /** Holds if the edge from `source` to `sink` is a taint sanitizer. */
+    /**
+     * DEPRECATED: Use `isSanitizerEdge` instead.
+     *
+     * Holds if the edge from `source` to `sink` is a taint sanitizer.
+     */
     predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) { none() }
 
-    /** Holds if the edge from `source` to `sink` is a taint sanitizer for data labelled with `lbl`. */
+    /**
+     * DEPRECATED: Use `isSanitizerEdge` instead.
+     *
+     * Holds if the edge from `source` to `sink` is a taint sanitizer for data labelled with `lbl`.
+     */
     predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink, DataFlow::FlowLabel lbl) {
       none()
     }
 
+    /** Holds if the edge from `pred` to `succ` is a taint sanitizer. */
+    predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) { none() }
+
+    /** Holds if the edge from `pred` to `succ` is a taint sanitizer for data labelled with `lbl`. */
+    predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel lbl) {
+      none()
+    }
+
     /**
      * Holds if data flow node `guard` can act as a sanitizer when appearing
      * in a condition.
@@ -74,16 +90,18 @@ module TaintTracking {
       isSanitizer(node)
     }
 
-    final override predicate isBarrier(DataFlow::Node source, DataFlow::Node sink) {
-      super.isBarrier(source, sink) or
-      isSanitizer(source, sink)
+    final override predicate isBarrierEdge(DataFlow::Node source, DataFlow::Node sink) {
+      super.isBarrierEdge(source, sink) or
+      isSanitizer(source, sink) or
+      isSanitizerEdge(source, sink)
     }
 
-    final override predicate isBarrier(
+    final override predicate isBarrierEdge(
       DataFlow::Node source, DataFlow::Node sink, DataFlow::FlowLabel lbl
     ) {
-      super.isBarrier(source, sink, lbl) or
-      isSanitizer(source, sink, lbl)
+      super.isBarrierEdge(source, sink, lbl) or
+      isSanitizer(source, sink, lbl) or
+      isSanitizerEdge(source, sink, lbl)
     }
 
     final override predicate isBarrierGuard(DataFlow::BarrierGuardNode guard) {

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

@@ -51,7 +51,7 @@ module ClientSideUrlRedirect {
       node instanceof Sanitizer
     }
 
-    override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) {
+    override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) {
       hostnameSanitizingPrefixEdge(source, sink)
     }
 

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

@@ -36,7 +36,7 @@ module InsecureRandomness {
       node instanceof Sanitizer
     }
 
-    override predicate isSanitizer(DataFlow::Node pred, DataFlow::Node succ) {
+    override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
       // stop propagation at the sinks to avoid double reporting
       pred instanceof Sink and
       // constrain succ

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

@@ -46,7 +46,7 @@ module RequestForgery {
       node instanceof Sanitizer
     }
 
-    override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) {
+    override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) {
       sanitizingPrefixEdge(source, sink)
     }
   }

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

@@ -38,7 +38,7 @@ module ServerSideUrlRedirect {
       node instanceof Sanitizer
     }
 
-    override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) {
+    override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) {
       hostnameSanitizingPrefixEdge(source, sink)
     }
 

javascript/ql/test/library-tests/InterProceduralFlow/DataFlowConfig.qll

@@ -17,7 +17,7 @@ class TestDataFlowConfiguration extends DataFlow::Configuration {
     )
   }
 
-  override predicate isBarrier(DataFlow::Node src, DataFlow::Node snk) {
+  override predicate isBarrierEdge(DataFlow::Node src, DataFlow::Node snk) {
     src = src and
     snk.asExpr().(PropAccess).getPropertyName() = "notTracked"
     or

javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql

@@ -17,7 +17,7 @@ class TestTaintTrackingConfiguration extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSanitizer(DataFlow::Node src, DataFlow::Node snk) {
+  override predicate isSanitizerEdge(DataFlow::Node src, DataFlow::Node snk) {
     src = src and
     snk.asExpr().(PropAccess).getPropertyName() = "notTracked"
     or