Python: Add support for more URL redirect sanitisers.

Since some sanitisers don't handle backslashes correctly, I updated the data-flow configuration to incorporate a flow state tracking whether or not backslashes have been eliminated or converted to forward slashes.

python/ql/lib/semmle/python/security/dataflow/UrlRedirectCustomizations.qll

@@ -16,6 +16,29 @@ private import semmle.python.dataflow.new.BarrierGuards
  * vulnerabilities, as well as extension points for adding your own.
  */
 module UrlRedirect {
+  /**
+   * A state value to track whether the untrusted data may contain backslashes.
+   */
+  abstract class FlowState extends string {
+    bindingset[this]
+    FlowState() { any() }
+  }
+
+  /**
+   * A state value signifying that the untrusted data may contain backslashes.
+   */
+  class MayContainBackslashes extends UrlRedirect::FlowState {
+    MayContainBackslashes() { this = "MayContainBackslashes" }
+  }
+
+  /**
+   * A state value signifying that any backslashes in the untrusted data have
+   * been eliminated, but no other sanitization has happened.
+   */
+  class NoBackslashes extends UrlRedirect::FlowState {
+    NoBackslashes() { this = "NoBackslashes" }
+  }
+
   /**
    * A data flow source for "URL redirection" vulnerabilities.
    */
@@ -29,7 +52,32 @@ module UrlRedirect {
   /**
    * A sanitizer for "URL redirection" vulnerabilities.
    */
-  abstract class Sanitizer extends DataFlow::Node { }
+  abstract class Sanitizer extends DataFlow::Node {
+    /**
+     * Holds if this sanitizer sanitizes flow in the given state.
+     *
+     * By default, sanitizers sanitize all flow, but some sanitiziers, for example,
+     * do not handle untrusted input that contains backslashes, so they only sanitize
+     * flow in the `NoBackslashes` state.
+     */
+    predicate sanitizes(FlowState state) { any() }
+  }
+
+  /**
+   * An additional flow step for "URL redirection" vulnerabilities.
+   */
+  abstract class AdditionalFlowStep extends DataFlow::Node {
+    /**
+     * Holds if there should be an additional flow step from `nodeFrom` in `stateFrom`
+     * to `nodeTo` in `stateTo`.
+     *
+     * For example, a call to `replace` that replaces backslashes with forward slashes
+     * takes flow from `MayContainBackslashes` to `NoBackslashes`.
+     */
+    abstract predicate step(
+      DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
+    );
+  }
 
   /**
    * A source of remote user input, considered as a flow source.
@@ -59,6 +107,30 @@ module UrlRedirect {
     }
   }
 
+  /**
+   * A call to replace backslashes with forward slashes or eliminates them
+   * altogether, considered as a partial sanitizer, as well as an additional
+   * flow step.
+   */
+  class ReplaceBackslashesSanitizer extends Sanitizer, AdditionalFlowStep, DataFlow::MethodCallNode {
+    ReplaceBackslashesSanitizer() {
+      this.getFunction().(DataFlow::AttrRead).getAttributeName() = "replace" and
+      this.getArg(0).asExpr().(StrConst).getText() = "\\" and
+      this.getArg(1).asExpr().(StrConst).getText() in ["/", ""]
+    }
+
+    override predicate sanitizes(FlowState state) { state instanceof MayContainBackslashes }
+
+    override predicate step(
+      DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
+    ) {
+      nodeFrom = this.getObject() and
+      stateFrom instanceof MayContainBackslashes and
+      nodeTo = this and
+      stateTo instanceof NoBackslashes
+    }
+  }
+
   /**
    * A comparison with a constant string, considered as a sanitizer-guard.
    */

python/ql/lib/semmle/python/security/dataflow/UrlRedirectQuery.qll

@@ -9,7 +9,7 @@
 private import python
 import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
-import UrlRedirectCustomizations::UrlRedirect
+import UrlRedirectCustomizations::UrlRedirect as UrlRedirect
 
 /**
  * DEPRECATED: Use `UrlRedirectFlow` module instead.
@@ -19,20 +19,48 @@ import UrlRedirectCustomizations::UrlRedirect
 deprecated class Configuration extends TaintTracking::Configuration {
   Configuration() { this = "UrlRedirect" }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof Source }
+  override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
+    source instanceof UrlRedirect::Source and state instanceof UrlRedirect::MayContainBackslashes
+  }
 
-  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+  override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
+    sink instanceof UrlRedirect::Sink and state instanceof UrlRedirect::FlowState
+  }
 
-  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+  override predicate isSanitizer(DataFlow::Node node, DataFlow::FlowState state) {
+    node.(UrlRedirect::Sanitizer).sanitizes(state)
+  }
+
+  override predicate isAdditionalTaintStep(
+    DataFlow::Node nodeFrom, DataFlow::FlowState stateFrom, DataFlow::Node nodeTo,
+    DataFlow::FlowState stateTo
+  ) {
+    any(UrlRedirect::AdditionalFlowStep a).step(nodeFrom, stateFrom, nodeTo, stateTo)
+  }
 }
 
-private module UrlRedirectConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { source instanceof Source }
+private module UrlRedirectConfig implements DataFlow::StateConfigSig {
+  class FlowState = UrlRedirect::FlowState;
 
-  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+  predicate isSource(DataFlow::Node source, FlowState state) {
+    source instanceof UrlRedirect::Source and state instanceof UrlRedirect::MayContainBackslashes
+  }
 
-  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+  predicate isSink(DataFlow::Node sink, FlowState state) {
+    sink instanceof UrlRedirect::Sink and
+    state = state
+  }
+
+  predicate isBarrier(DataFlow::Node node, FlowState state) {
+    node.(UrlRedirect::Sanitizer).sanitizes(state)
+  }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
+  ) {
+    any(UrlRedirect::AdditionalFlowStep a).step(nodeFrom, stateFrom, nodeTo, stateTo)
+  }
 }
 
 /** Global taint-tracking for detecting "URL redirection" vulnerabilities. */
-module UrlRedirectFlow = TaintTracking::Global<UrlRedirectConfig>;
+module UrlRedirectFlow = TaintTracking::GlobalWithState<UrlRedirectConfig>;