Apply suggestions from code review

Co-authored-by: Taus <[email protected]>

Author: 2 people

python/ql/lib/semmle/python/frameworks/Django.qll

@@ -2928,5 +2928,10 @@ module PrivateDjango {
     DjangoAllowedUrl() {
       this = DataFlow::BarrierGuard<djangoUrlHasAllowedHostAndScheme/3>::getABarrierNode()
     }
+
+    override predicate sanitizes(UrlRedirect::FlowState state) {
+      // sanitize all flow states
+      any()
+    }
   }
 }

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

@@ -55,12 +55,8 @@ module UrlRedirect {
   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() }
+    abstract predicate sanitizes(FlowState state);
   }
 
   /**
@@ -105,16 +101,23 @@ module UrlRedirect {
         string_concat.getRight() = this.asCfgNode()
       )
     }
+
+    override predicate sanitizes(FlowState state) {
+      // sanitize all flow states
+      any()
+    }
   }
 
   /**
-   * A call to replace backslashes with forward slashes or eliminates them
+   * A call that replaces 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 {
+    DataFlow::Node receiver;
+
     ReplaceBackslashesSanitizer() {
-      this.getFunction().(DataFlow::AttrRead).getAttributeName() = "replace" and
+      this.calls(receiver, "replace") and
       this.getArg(0).asExpr().(StrConst).getText() = "\\" and
       this.getArg(1).asExpr().(StrConst).getText() in ["/", ""]
     }
@@ -124,7 +127,7 @@ module UrlRedirect {
     override predicate step(
       DataFlow::Node nodeFrom, FlowState stateFrom, DataFlow::Node nodeTo, FlowState stateTo
     ) {
-      nodeFrom = this.getObject() and
+      nodeFrom = receiver and
       stateFrom instanceof MayContainBackslashes and
       nodeTo = this and
       stateTo instanceof NoBackslashes
@@ -134,5 +137,10 @@ module UrlRedirect {
   /**
    * A comparison with a constant string, considered as a sanitizer-guard.
    */
-  class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
+  class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier {
+    override predicate sanitizes(FlowState state) {
+      // sanitize all flow states
+      any()
+    }
+  }
 }

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

@@ -48,7 +48,7 @@ private module UrlRedirectConfig implements DataFlow::StateConfigSig {
 
   predicate isSink(DataFlow::Node sink, FlowState state) {
     sink instanceof UrlRedirect::Sink and
-    state = state
+    exists(state)
   }
 
   predicate isBarrier(DataFlow::Node node, FlowState state) {