From 5fd3594f5fda5773e889bf6309e6a75f4ad101d3 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Fri, 25 Aug 2023 22:15:43 +0200
Subject: [PATCH] Python: Move TimingAttack.qll to new dataflow API

---
 .../semmle/python/security/TimingAttack.qll   | 43 ++++++++-----------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/python/ql/src/experimental/semmle/python/security/TimingAttack.qll b/python/ql/src/experimental/semmle/python/security/TimingAttack.qll
index 4df7752e64d..37d3a35158d 100644
--- a/python/ql/src/experimental/semmle/python/security/TimingAttack.qll
+++ b/python/ql/src/experimental/semmle/python/security/TimingAttack.qll
@@ -164,9 +164,7 @@ class NonConstantTimeComparisonSink extends DataFlow::Node {
 
   /** Holds if remote user input was used in the comparison. */
   predicate includesUserInput() {
-    exists(UserInputInComparisonConfig config |
-      config.hasFlowTo(DataFlow2::exprNode(anotherParameter))
-    )
+    UserInputInComparisonFlow::flowTo(DataFlow2::exprNode(anotherParameter))
   }
 }
 
@@ -177,9 +175,7 @@ class SecretSource extends DataFlow::Node {
   SecretSource() { secret = this.asExpr() }
 
   /** Holds if the secret was deliverd by remote user. */
-  predicate includesUserInput() {
-    exists(UserInputSecretConfig config | config.hasFlowTo(DataFlow2::exprNode(secret)))
-  }
+  predicate includesUserInput() { UserInputSecretFlow::flowTo(DataFlow2::exprNode(secret)) }
 }
 
 /** A string for `match` that identifies strings that look like they represent secret data. */
@@ -267,23 +263,21 @@ private string sensitiveheaders() {
 /**
  * A config that tracks data flow from remote user input to Variable that hold sensitive info
  */
-class UserInputSecretConfig extends TaintTracking::Configuration {
-  UserInputSecretConfig() { this = "UserInputSecretConfig" }
+module UserInputSecretConfig  implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof CredentialExpr }
+  predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof CredentialExpr }
 }
 
+module UserInputSecretFlow = TaintTracking::Global<UserInputSecretConfig>;
+
 /**
  * A config that tracks data flow from remote user input to Equality test
  */
-class UserInputInComparisonConfig extends TaintTracking2::Configuration {
-  UserInputInComparisonConfig() { this = "UserInputInComparisonConfig" }
+module UserInputInComparisonConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(Compare cmp, Expr left, Expr right, Cmpop cmpop |
       cmpop.getSymbol() = ["==", "in", "is not", "!="] and
       cmp.compares(left, cmpop, right) and
@@ -292,15 +286,15 @@ class UserInputInComparisonConfig extends TaintTracking2::Configuration {
   }
 }
 
+module UserInputInComparisonFlow = TaintTracking::Global<UserInputInComparisonConfig>;
+
 /**
  * A configuration tracing flow from  a client Secret obtained by an HTTP header to a len() function.
  */
-private class ExcludeLenFunc extends TaintTracking2::Configuration {
-  ExcludeLenFunc() { this = "ExcludeLenFunc" }
+private module ExcludeLenFuncConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof ClientSuppliedSecret }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof ClientSuppliedSecret }
-
-  override predicate isSink(DataFlow::Node sink) {
+  predicate isSink(DataFlow::Node sink) {
     exists(Call call |
       call.getFunc().(Name).getId() = "len" and
       sink.asExpr() = call.getArg(0)
@@ -308,6 +302,8 @@ private class ExcludeLenFunc extends TaintTracking2::Configuration {
   }
 }
 
+module ExcludeLenFuncFlow = TaintTracking::Global<ExcludeLenFuncConfig>;
+
 /**
  * Holds if there is a fast-fail check.
  */
@@ -343,8 +339,7 @@ class CompareSink extends DataFlow::Node {
    * Holds if there is a flow to len().
    */
   predicate flowtolen() {
-    exists(ExcludeLenFunc config, DataFlow2::PathNode source, DataFlow2::PathNode sink |
-      config.hasFlowPath(source, sink)
-    )
+    // TODO: Fly by comment: I don't understand this code at all, seems very strange.
+    ExcludeLenFuncFlow::flowPath(_, _)
   }
 }