Merge pull request #18175 from asgerf/jss/documentation

JS: Update data flow documentation and tutorials for JavaScript

Author: asgerf

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

@@ -204,58 +204,45 @@ data flow solver that can check whether there is (global) data flow from a sourc
 Optionally, configurations may specify extra data flow edges to be added to the data flow graph, and may also specify  `barriers`. Barriers are data flow nodes or edges through
 which data should not be tracked for the purposes of this analysis.
 
-To define a configuration, extend the class ``DataFlow::Configuration`` as follows:
+To define a configuration, add a module that implements the signature ``DataFlow::ConfigSig`` and pass it to ``DataFlow::Global`` as follows:
 
 .. code-block:: ql
 
-  class MyDataFlowConfiguration extends DataFlow::Configuration {
-    MyDataFlowConfiguration() { this = "MyDataFlowConfiguration" }
+  module MyAnalysisConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) { /* ... */ }
 
-    override predicate isSource(DataFlow::Node source) { /* ... */ }
+    predicate isSink(DataFlow::Node sink) { /* ... */ }
 
-    override predicate isSink(DataFlow::Node sink) { /* ... */ }
-
-    // optional overrides:
-    override predicate isBarrier(DataFlow::Node nd) { /* ... */ }
-    override predicate isBarrierEdge(DataFlow::Node pred, DataFlow::Node succ) { /* ... */ }
-    override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { /* ... */ }
+    // optional predicates:
+    predicate isBarrier(DataFlow::Node nd) { /* ... */ }
+    predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { /* ... */ }
   }
 
-The characteristic predicate ``MyDataFlowConfiguration()`` defines the name of the configuration, so ``"MyDataFlowConfiguration"`` should be replaced by a suitable
-name describing your particular analysis configuration.
+  module MyAnalysisFlow = DataFlow::Global<MyAnalysisConfig>
 
-The data flow analysis is performed using the predicate ``hasFlow(source, sink)``:
+The data flow analysis is performed using the predicate ``MyAnalysisFlow::flow(source, sink)``:
 
 .. code-block:: ql
 
-   from MyDataFlowConfiguration dataflow, DataFlow::Node source, DataFlow::Node sink
-   where dataflow.hasFlow(source, sink)
+   from DataFlow::Node source, DataFlow::Node sink
+   where MyAnalysisFlow::flow(source, sink)
    select source, "Data flow from $@ to $@.", source, source.toString(), sink, sink.toString()
 
 Using global taint tracking
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Global taint tracking extends global data flow with additional non-value-preserving steps, such as flow through string-manipulating operations. To use it, simply extend
-``TaintTracking::Configuration`` instead of ``DataFlow::Configuration``:
+Global taint tracking extends global data flow with additional non-value-preserving steps, such as flow through string-manipulating operations. To use it, simply
+use ``TaintTracking::Global<...>`` instead of ``DataFlow::Global<...>``:
 
 .. code-block:: ql
 
-  class MyTaintTrackingConfiguration extends TaintTracking::Configuration {
-    MyTaintTrackingConfiguration() { this = "MyTaintTrackingConfiguration" }
-
-    override predicate isSource(DataFlow::Node source) { /* ... */ }
-
-    override predicate isSink(DataFlow::Node sink) { /* ... */ }
+  module MyAnalysisConfig implements DataFlow::ConfigSig {
+    /* ... */
   }
 
-Analogous to ``isAdditionalFlowStep``, there is a predicate ``isAdditionalTaintStep`` that you can override to specify custom flow steps to consider in the analysis.
-Instead of the ``isBarrier`` and ``isBarrierEdge`` predicates, the taint tracking configuration includes ``isSanitizer`` and ``isSanitizerEdge`` predicates that specify
-data flow nodes or edges that act as taint sanitizers and hence stop flow from a source to a sink.
+  module MyAnalysisFlow = TaintTracking::Global<MyAnalysisConfig>
 
-Similar to global data flow, the characteristic predicate ``MyTaintTrackingConfiguration()`` defines the unique name of the configuration, so ``"MyTaintTrackingConfiguration"``
-should be replaced by an appropriate descriptive name.
-
-The taint tracking analysis is again performed using the predicate ``hasFlow(source, sink)``.
+The taint tracking analysis is again performed using the predicate ``MyAnalysisFlow::flow(source, sink)``.
 
 Examples
 ~~~~~~~~
@@ -267,20 +254,20 @@ time using global taint tracking.
 
   import javascript
 
-  class CommandLineFileNameConfiguration extends TaintTracking::Configuration {
-    CommandLineFileNameConfiguration() { this = "CommandLineFileNameConfiguration" }
-
-    override predicate isSource(DataFlow::Node source) {
+  module CommandLineFileNameConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) {
       DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead() = source
     }
 
-    override predicate isSink(DataFlow::Node sink) {
+    predicate isSink(DataFlow::Node sink) {
       DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink
     }
   }
 
-  from CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink
-  where cfg.hasFlow(source, sink)
+  module CommandLineFileNameFlow = TaintTracking::Global<CommandLineFileNameConfig>;
+
+  from DataFlow::Node source, DataFlow::Node sink
+  where CommandLineFileNameFlow::flow(source, sink)
   select source, sink
 
 This query will now find flows that involve inter-procedural steps, like in the following example (where the individual steps have been marked with comments
@@ -325,15 +312,15 @@ with an error if it does not. We could then use that function in ``readFileHelpe
   }
 
 For the purposes of our above analysis, ``checkPath`` is a `sanitizer`: its output is always untainted, even if its input is tainted. To model this
-we can add an override of ``isSanitizer`` to our taint-tracking configuration like this:
+we can add an ``isBarrier`` predicate to our taint-tracking configuration like this:
 
 .. code-block:: ql
 
-  class CommandLineFileNameConfiguration extends TaintTracking::Configuration {
+  module CommandLineFileNameConfig implements DataFlow::ConfigSig {
 
     // ...
 
-    override predicate isSanitizer(DataFlow::Node nd) {
+    predicate isBarrier(DataFlow::Node nd) {
       nd.(DataFlow::CallNode).getCalleeName() = "checkPath"
     }
   }
@@ -359,36 +346,36 @@ Note that ``checkPath`` is now no longer a sanitizer in the sense described abov
 through ``checkPath`` any more. The flow is, however, `guarded` by ``checkPath`` in the sense that the expression ``checkPath(p)`` has to evaluate
 to ``true`` (or, more precisely, to a truthy value) in order for the flow to happen.
 
-Such sanitizer guards can be supported by defining a new subclass of ``TaintTracking::SanitizerGuardNode`` and overriding the predicate
-``isSanitizerGuard`` in the taint-tracking configuration class to add all instances of this class as sanitizer guards to the configuration.
+Such sanitizer guards can be supported by defining a class with a ``blocksExpr`` predicate and using the `DataFlow::MakeBarrierGuard`` module
+to implement the ``isBarrier`` predicate.
 
-For our above example, we would begin by defining a subclass of ``SanitizerGuardNode`` that identifies guards of the form ``checkPath(...)``:
+For our above example, we would begin by defining a subclass of ``DataFlow::CallNode`` that identifies guards of the form ``checkPath(...)``:
 
 .. code-block:: ql
 
-  class CheckPathSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {
+  class CheckPathSanitizerGuard extends DataFlow::CallNode {
     CheckPathSanitizerGuard() { this.getCalleeName() = "checkPath" }
 
-    override predicate sanitizes(boolean outcome, Expr e) {
+    predicate blocksExpr(boolean outcome, Expr e) {
       outcome = true and
-      e = getArgument(0).asExpr()
+      e = this.getArgument(0).asExpr()
     }
   }
 
-The characteristic predicate of this class checks that the sanitizer guard is a call to a function named ``checkPath``. The overriding definition
-of ``sanitizes`` says such a call sanitizes its first argument (that is, ``getArgument(0)``) if it evaluates to ``true`` (or rather, a truthy
+The characteristic predicate of this class checks that the sanitizer guard is a call to a function named ``checkPath``. The definition
+of ``blocksExpr`` says such a call sanitizes its first argument (that is, ``getArgument(0)``) if it evaluates to ``true`` (or rather, a truthy
 value).
 
-Now we can override ``isSanitizerGuard`` to add these sanitizer guards to our configuration:
+Now we can implement ``isBarrier`` to add this sanitizer guard to our configuration:
 
 .. code-block:: ql
 
-  class CommandLineFileNameConfiguration extends TaintTracking::Configuration {
+  module CommandLineFileNameConfig implements DataFlow::ConfigSig {
 
     // ...
 
-    override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) {
-      nd instanceof CheckPathSanitizerGuard
+    predicate isBarrier(DataFlow::Node node) {
+      node = DataFlow::MakeBarrierGuard<CheckPathSanitizerGuard>::getABarrierNode()
     }
   }
 
@@ -399,7 +386,7 @@ reach there if ``checkPath(p)`` evaluates to a truthy value. Consequently, there
 Additional taint steps
 ~~~~~~~~~~~~~~~~~~~~~~
 
-Sometimes the default data flow and taint steps provided by ``DataFlow::Configuration`` and ``TaintTracking::Configuration`` are not sufficient
+Sometimes the default data flow and taint steps provided by the data flow library are not sufficient
 and we need to add additional flow or taint steps to our configuration to make it find the expected flow. For example, this can happen because
 the analyzed program uses a function from an external library whose source code is not available to the analysis, or because it uses a function
 that is too difficult to analyze.
@@ -420,16 +407,16 @@ to resolve any symlinks in the path ``p`` before passing it to ``readFile``:
 Resolving symlinks does not make an unsafe path any safer, so we would still like our query to flag this, but since the standard library does
 not have a model of ``resolve-symlinks`` it will no longer return any results.
 
-We can fix this quite easily by adding an overriding definition of the ``isAdditionalTaintStep`` predicate to our configuration, introducing an
+We can fix this quite easily by adding a definition of the ``isAdditionalFlowStep`` predicate to our configuration, introducing an
 additional taint step from the first argument of ``resolveSymlinks`` to its result:
 
 .. code-block:: ql
 
-  class CommandLineFileNameConfiguration extends TaintTracking::Configuration {
+  module CommandLineFileNameConfig implements DataFlow::ConfigSig {
 
     // ...
 
-    override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
       exists(DataFlow::CallNode c |
         c = DataFlow::moduleImport("resolve-symlinks").getACall() and
         pred = c.getArgument(0) and
@@ -494,18 +481,18 @@ Exercise 2
 
   import javascript
 
-  class HardCodedTagNameConfiguration extends DataFlow::Configuration {
-    HardCodedTagNameConfiguration() { this = "HardCodedTagNameConfiguration" }
-
-    override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof ConstantString }
+  module HardCodedTagNameConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) { source.asExpr() instanceof ConstantString }
 
-    override predicate isSink(DataFlow::Node sink) {
+    predicate isSink(DataFlow::Node sink) {
       sink = DataFlow::globalVarRef("document").getAMethodCall("createElement").getArgument(0)
     }
   }
 
-  from HardCodedTagNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink
-  where cfg.hasFlow(source, sink)
+  module HardCodedTagNameFlow = DataFlow::Global<HardCodedTagNameConfig>;
+
+  from DataFlow::Node source, DataFlow::Node sink
+  where HardCodedTagNameFlow::flow(source, sink)
   select source, sink
 
 Exercise 3
@@ -540,18 +527,18 @@ Exercise 4
     }
   }
 
-  class HardCodedTagNameConfiguration extends DataFlow::Configuration {
-    HardCodedTagNameConfiguration() { this = "HardCodedTagNameConfiguration" }
+  module HardCodedTagNameConfig implements DataFlow::ConfigSig {
+    predicate isSource(DataFlow::Node source) { source instanceof ArrayEntryCallResult }
 
-    override predicate isSource(DataFlow::Node source) { source instanceof ArrayEntryCallResult }
-
-    override predicate isSink(DataFlow::Node sink) {
+    predicate isSink(DataFlow::Node sink) {
       sink = DataFlow::globalVarRef("document").getAMethodCall("createElement").getArgument(0)
     }
   }
 
-  from HardCodedTagNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink
-  where cfg.hasFlow(source, sink)
+  module HardCodedTagNameFlow = DataFlow::Global<HardCodedTagNameConfig>;
+
+  from DataFlow::Node source, DataFlow::Node sink
+  where HardCodedTagNameFlow::flow(source, sink)
   select source, sink
 
 Further reading

docs/codeql/codeql-language-guides/codeql-library-for-javascript.rst

@@ -700,55 +700,44 @@ The data flow graph-based analyses described so far are all intraprocedural: the
 
 We distinguish here between data flow proper, and *taint tracking*: the latter not only considers value-preserving flow (such as from variable definitions to uses), but also cases where one value influences ("taints") another without determining it entirely. For example, in the assignment ``s2 = s1.substring(i)``, the value of ``s1`` influences the value of ``s2``, because ``s2`` is assigned a substring of ``s1``. In general, ``s2`` will not be assigned ``s1`` itself, so there is no data flow from ``s1`` to ``s2``, but ``s1`` still taints ``s2``.
 
-It is a common pattern that we wish to specify data flow or taint analysis in terms of its *sources* (where flow starts), *sinks* (where it should be tracked), and *barriers* or *sanitizers* (where flow is interrupted). Sanitizers they are very common in security analyses: for example, an analysis that tracks the flow of untrusted user input into, say, a SQL query has to keep track of code that validates the input, thereby making it safe to use. Such a validation step is an example of a sanitizer.
+It is a common pattern that we wish to specify data flow or taint analysis in terms of its *sources* (where flow starts), *sinks* (where it should be tracked), and *barriers* (also called *sanitizers*) where flow is interrupted. Sanitizers they are very common in security analyses: for example, an analysis that tracks the flow of untrusted user input into, say, a SQL query has to keep track of code that validates the input, thereby making it safe to use. Such a validation step is an example of a sanitizer.
 
-The classes ``DataFlow::Configuration`` and ``TaintTracking::Configuration`` allow specifying a data flow or taint analysis, respectively, by overriding the following predicates:
+A module implementing the signature `DataFlow::ConfigSig` may specify a data flow or taint analysis by implementing the following predicates:
 
 -  ``isSource(DataFlow::Node nd)`` selects all nodes ``nd`` from where flow tracking starts.
 -  ``isSink(DataFlow::Node nd)`` selects all nodes ``nd`` to which the flow is tracked.
--  ``isBarrier(DataFlow::Node nd)`` selects all nodes ``nd`` that act as a barrier for data flow; ``isSanitizer`` is the corresponding predicate for taint tracking configurations.
--  ``isBarrierEdge(DataFlow::Node src, DataFlow::Node trg)`` is a variant of ``isBarrier(nd)`` that allows specifying barrier *edges* in addition to barrier nodes; again, ``isSanitizerEdge`` is the corresponding predicate for taint tracking;
--  ``isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg)`` allows specifying custom additional flow steps for this analysis; ``isAdditionalTaintStep`` is the corresponding predicate for taint tracking configurations.
+-  ``isBarrier(DataFlow::Node nd)`` selects all nodes ``nd`` that act as a barrier/sanitizer for data flow.
+-  ``isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg)`` allows specifying custom additional flow steps for this analysis.
 
-Since for technical reasons both ``Configuration`` classes are subtypes of ``string``, you have to choose a unique name for each flow configuration and equate ``this`` with it in the characteristic predicate (as in the example below).
-
-The predicate ``Configuration.hasFlow`` performs the actual flow tracking, starting at a source and looking for flow to a sink that does not pass through a barrier node or edge.
+Such a module can be passed to ``DataFlow::Global<...>``. This will produce a module with a ``flow`` predicate that performs the actual flow tracking, starting at a source and looking for flow to a sink that does not pass through a barrier node.
 
 For example, suppose that we are developing an analysis to find hard-coded passwords. We might write a simple query that looks for string constants flowing into variables named ``"password"``.
 
 .. code-block:: ql
 
    import javascript
 
-   class PasswordTracker extends DataFlow::Configuration {
-       PasswordTracker() {
-           // unique identifier for this configuration
-           this = "PasswordTracker"
-       }
+   module PasswordConfig implements DataFlow::ConfigSig {
+       predicate isSource(DataFlow::Node nd) { nd.asExpr() instanceof StringLiteral }
 
-       override predicate isSource(DataFlow::Node nd) {
-           nd.asExpr() instanceof StringLiteral
-       }
-
-       override predicate isSink(DataFlow::Node nd) {
-           passwordVarAssign(_, nd)
-       }
+       predicate isSink(DataFlow::Node nd) { passwordVarAssign(_, nd) }
+   }
 
-       predicate passwordVarAssign(Variable v, DataFlow::Node nd) {
-          v.getAnAssignedExpr() = nd.asExpr() and
-          v.getName().toLowerCase() = "password"
-       }
+   predicate passwordVarAssign(Variable v, DataFlow::Node nd) {
+       v.getAnAssignedExpr() = nd.asExpr() and
+       v.getName().toLowerCase() = "password"
    }
 
-Now we can rephrase our query to use ``Configuration.hasFlow``:
+   module PasswordFlow = DataFlow::Global<PasswordConfig>;
+
+Now we can rephrase our query to use ``PasswordFlow::flow``:
 
 .. code-block:: ql
 
-   from PasswordTracker pt, DataFlow::Node source, DataFlow::Node sink, Variable v
-   where pt.hasFlow(source, sink) and pt.passwordVarAssign(v, sink)
+   from DataFlow::Node source, DataFlow::Node sink, Variable v
+   where PasswordFlow::flow(_, sink) and passwordVarAssign(v, sink)
    select sink, "Password variable " + v + " is assigned a constant string."
 
-
 Syntax errors
 ~~~~~~~~~~~~~
 

docs/codeql/codeql-language-guides/data-flow-cheat-sheet-for-javascript.rst

@@ -16,18 +16,17 @@ Use the following template to create a taint tracking path query:
      * @kind path-problem
      */
     import javascript
-    import DataFlow
-    import DataFlow::PathGraph
-
-    class MyConfig extends TaintTracking::Configuration {
-      MyConfig() { this = "MyConfig" }
-      override predicate isSource(Node node) { ... }
-      override predicate isSink(Node node) { ... }
-      override predicate isAdditionalTaintStep(Node pred, Node succ) { ... }
+
+    module MyConfig implements DataFlow::ConfigSig {
+      predicate isSource(DataFlow::Node node) { ... }
+      predicate isSink(DataFlow::Node node) { ... }
+      predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { ... }
     }
 
-    from MyConfig cfg, PathNode source, PathNode sink
-    where cfg.hasFlowPath(source, sink)
+    module MyFlow = TaintTracking::Global<MyConfig>;
+
+    from MyFlow::PathNode source, MyFlow::PathNode sink
+    where MyFlow::flowPath(source, sink)
     select sink.getNode(), source, sink, "taint from $@.", source.getNode(), "here"
 
 This query reports flow paths which: