Merge pull request #18265 from asgerf/jss/flow-labels2

JS: Migrate all queries to proper flow states and deprecate FlowLabel

Author: asgerf

docs/codeql/codeql-language-guides/analyzing-data-flow-in-javascript-and-typescript.rst

@@ -416,11 +416,11 @@ additional taint step from the first argument of ``resolveSymlinks`` to its resu
 
     // ...
 
-    predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+    predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
       exists(DataFlow::CallNode c |
         c = DataFlow::moduleImport("resolve-symlinks").getACall() and
-        pred = c.getArgument(0) and
-        succ = c
+        node1 = c.getArgument(0) and
+        node2 = c
       )
     }
   }
@@ -431,11 +431,11 @@ to wrap it in a new subclass of ``TaintTracking::SharedTaintStep`` like this:
 .. code-block:: ql
 
   class StepThroughResolveSymlinks extends TaintTracking::SharedTaintStep {
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+    override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
       exists(DataFlow::CallNode c |
         c = DataFlow::moduleImport("resolve-symlinks").getACall() and
-        pred = c.getArgument(0) and
-        succ = c
+        node1 = c.getArgument(0) and
+        node2 = c
       )
     }
   }

javascript/ql/examples/queries/dataflow/BackendIdor/BackendIdor.ql

@@ -18,9 +18,9 @@ module IdorTaintConfig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node node) { exists(ClientRequest req | node = req.getADataNode()) }
 
-  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     // Step from x -> { userId: x }
-    succ.(DataFlow::SourceNode).getAPropertyWrite("userId").getRhs() = pred
+    node2.(DataFlow::SourceNode).getAPropertyWrite("userId").getRhs() = node1
   }
 
   predicate isBarrier(DataFlow::Node node) {

javascript/ql/examples/queries/dataflow/InformationDisclosure/InformationDisclosure.ql

@@ -37,16 +37,16 @@ module AuthKeyTrackingConfig implements DataFlow::ConfigSig {
     )
   }
 
-  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     // Step into objects: x -> { f: x }
-    succ.(DataFlow::SourceNode).getAPropertyWrite().getRhs() = pred
+    node2.(DataFlow::SourceNode).getAPropertyWrite().getRhs() = node1
     or
     // Step through JSON serialization: x -> JSON.stringify(x)
     // Note: TaintTracking::Configuration includes this step by default, but not DataFlow::Configuration
     exists(DataFlow::CallNode call |
       call = DataFlow::globalVarRef("JSON").getAMethodCall("stringify") and
-      pred = call.getArgument(0) and
-      succ = call
+      node1 = call.getArgument(0) and
+      node2 = call
     )
   }
 }

javascript/ql/lib/semmle/javascript/dataflow/AdditionalFlowSteps.qll

@@ -147,10 +147,12 @@ class LegacyFlowStep extends Unit {
   predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() }
 
   /**
+   * DEPRECATED. The `FlowLabel` class and steps involving flow labels are no longer used by any queries.
+   *
    * Holds if `pred` → `succ` should be considered a data flow edge
    * transforming values with label `predlbl` to have label `succlbl`.
    */
-  predicate step(
+  deprecated predicate step(
     DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl,
     DataFlow::FlowLabel succlbl
   ) {
@@ -199,11 +201,13 @@ module LegacyFlowStep {
   }
 
   /**
+   * DEPRECATED. The `FlowLabel` class and steps involving flow labels are no longer used by any queries.
+   *
    * Holds if `pred` → `succ` should be considered a data flow edge
    * transforming values with label `predlbl` to have label `succlbl`.
    */
   cached
-  predicate step(
+  deprecated predicate step(
     DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl,
     DataFlow::FlowLabel succlbl
   ) {
@@ -273,10 +277,12 @@ class SharedFlowStep extends Unit {
   predicate step(DataFlow::Node pred, DataFlow::Node succ) { none() }
 
   /**
+   * DEPRECATED. The `FlowLabel` class and steps involving flow labels are no longer used by any queries.
+   *
    * Holds if `pred` → `succ` should be considered a data flow edge
    * transforming values with label `predlbl` to have label `succlbl`.
    */
-  predicate step(
+  deprecated predicate step(
     DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl,
     DataFlow::FlowLabel succlbl
   ) {
@@ -353,10 +359,12 @@ module SharedFlowStep {
 
   // The following are aliases for old step predicates that have no corresponding predicate in AdditionalFlowStep
   /**
+   * DEPRECATED. The `FlowLabel` class and steps involving flow labels are no longer used by any queries.
+   *
    * Holds if `pred` → `succ` should be considered a data flow edge
    * transforming values with label `predlbl` to have label `succlbl`.
    */
-  predicate step(
+  deprecated predicate step(
     DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl,
     DataFlow::FlowLabel succlbl
   ) {

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

@@ -295,6 +295,9 @@ deprecated private predicate isBarrierGuardInternal(
 }
 
 /**
+ * DEPRECATED. Use a query-specific `FlowState` class instead.
+ * See [guide on using flow state](https://codeql.github.com/docs/codeql-language-guides/using-flow-labels-for-precise-data-flow-analysis) for more details.
+ *
  * A label describing the kind of information tracked by a flow configuration.
  *
  * There are two standard labels "data" and "taint".
@@ -303,7 +306,7 @@ deprecated private predicate isBarrierGuardInternal(
  * - "taint" additionally permits flow through transformations such as string operations,
  *   and is the default flow source for a `TaintTracking::Configuration`.
  */
-abstract class FlowLabel extends string {
+abstract deprecated class FlowLabel extends string {
   bindingset[this]
   FlowLabel() { any() }
 
@@ -341,7 +344,7 @@ deprecated class StandardFlowLabel extends FlowLabel {
   StandardFlowLabel() { this = "data" or this = "taint" }
 }
 
-module FlowLabel {
+deprecated module FlowLabel {
   /**
    * Gets the standard flow label for describing values that directly originate from a flow source.
    */

javascript/ql/lib/semmle/javascript/dataflow/internal/BarrierGuards.qll

@@ -36,17 +36,19 @@ module MakeBarrierGuard<BarrierGuardSig BaseGuard> {
   }
 }
 
-private signature class LabeledBarrierGuardSig extends DataFlow::Node {
-  /**
-   * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`.
-   */
-  predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label);
+deprecated private module DeprecationWrapper {
+  signature class LabeledBarrierGuardSig extends DataFlow::Node {
+    /**
+     * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`.
+     */
+    predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label);
+  }
 }
 
 /**
  * Converts a barrier guard class to a set of nodes to include in an implementation of `isBarrier(node, label)`.
  */
-module MakeLabeledBarrierGuard<LabeledBarrierGuardSig BaseGuard> {
+deprecated module MakeLabeledBarrierGuard<DeprecationWrapper::LabeledBarrierGuardSig BaseGuard> {
   final private class FinalBaseGuard = BaseGuard;
 
   private class Adapter extends FinalBaseGuard {

javascript/ql/lib/semmle/javascript/dataflow/internal/FlowSteps.qll

@@ -545,9 +545,9 @@ class Boolean extends boolean {
 /**
  * A summary of an inter-procedural data flow path.
  */
-newtype TPathSummary =
+deprecated newtype TPathSummary =
   /** A summary of an inter-procedural data flow path. */
-  MkPathSummary(Boolean hasReturn, Boolean hasCall, FlowLabel start, FlowLabel end)
+  deprecated MkPathSummary(Boolean hasReturn, Boolean hasCall, FlowLabel start, FlowLabel end)
 
 /**
  * A summary of an inter-procedural data flow path.
@@ -560,7 +560,7 @@ newtype TPathSummary =
  * We only want to build properly matched call/return sequences, so if a path has both
  * call steps and return steps, all return steps must precede all call steps.
  */
-class PathSummary extends TPathSummary {
+deprecated class PathSummary extends TPathSummary {
   Boolean hasReturn;
   Boolean hasCall;
   FlowLabel start;
@@ -634,7 +634,7 @@ class PathSummary extends TPathSummary {
   }
 }
 
-module PathSummary {
+deprecated module PathSummary {
   /**
    * Gets a summary describing a path without any calls or returns.
    */

javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll

@@ -0,0 +1,90 @@
+/**
+ * Contains a class with flow states that are used by multiple queries.
+ */
+
+private import javascript
+private import TaintedUrlSuffixCustomizations
+private import TaintedObjectCustomizations
+
+private newtype TFlowState =
+  TTaint() or
+  TTaintedUrlSuffix() or
+  TTaintedPrefix() or
+  TTaintedObject()
+
+/**
+ * A flow state indicating which part of a value is tainted.
+ */
+class FlowState extends TFlowState {
+  /**
+   * Holds if this represents a value that is considered entirely tainted, except the first character
+   * might not be user-controlled.
+   */
+  predicate isTaint() { this = TTaint() }
+
+  /**
+   * Holds if this represents a URL whose fragment and/or query parts are considered tainted.
+   */
+  predicate isTaintedUrlSuffix() { this = TTaintedUrlSuffix() }
+
+  /**
+   * Holds if this represents a string whose prefix is known to be tainted.
+   */
+  predicate isTaintedPrefix() { this = TTaintedPrefix() }
+
+  /**
+   * Holds if this represents a deeply tainted object, such as a JSON object
+   * parsed from user-controlled data.
+   */
+  predicate isTaintedObject() { this = TTaintedObject() }
+
+  /** Gets a string representation of this flow state. */
+  string toString() {
+    this.isTaint() and result = "taint"
+    or
+    this.isTaintedUrlSuffix() and result = "tainted-url-suffix"
+    or
+    this.isTaintedPrefix() and result = "tainted-prefix"
+    or
+    this.isTaintedObject() and result = "tainted-object"
+  }
+
+  /** DEPRECATED. Gets the corresponding flow label. */
+  deprecated DataFlow::FlowLabel toFlowLabel() {
+    this.isTaint() and result.isTaint()
+    or
+    this.isTaintedUrlSuffix() and result = TaintedUrlSuffix::label()
+    or
+    this.isTaintedPrefix() and result = "PrefixString"
+    or
+    this.isTaintedObject() and result = TaintedObject::label()
+  }
+}
+
+/** Convenience predicates for working with common flow states. */
+module FlowState {
+  /**
+   * Gets the flow state representing a value that is considered entirely tainted, except the first character
+   * might not be user-controlled.
+   */
+  FlowState taint() { result.isTaint() }
+
+  /**
+   * Gets the flow state representing a URL whose fragment and/or query parts are considered tainted.
+   */
+  FlowState taintedUrlSuffix() { result.isTaintedUrlSuffix() }
+
+  /**
+   * Gets the flow state representing a string whose prefix is known to be tainted.
+   */
+  FlowState taintedPrefix() { result.isTaintedPrefix() }
+
+  /**
+   * Gets the flow state representing a deeply tainted object, such as a JSON object
+   * parsed from user-controlled data.
+   */
+  FlowState taintedObject() { result.isTaintedObject() }
+
+  /** DEPRECATED. Gets the flow state corresponding to `label`. */
+  deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label }
+}