From 0be280c6085eb56655ea59274a4f9f3cfbb86bd5 Mon Sep 17 00:00:00 2001
From: Rasmus Wriedt Larsen <rasmuswl@github.com>
Date: Tue, 29 Jun 2021 18:55:04 +0200
Subject: [PATCH] Python: Port `py/sql-injection` to use proper source/sink
 customization

---
 .../ql/src/Security/CWE-089/SqlInjection.ql   |  2 +-
 .../python/security/dataflow/SqlInjection.qll | 42 +++++++++-----
 .../dataflow/SqlInjectionCustomizations.qll   | 55 +++++++++++++++++++
 3 files changed, 85 insertions(+), 14 deletions(-)
 create mode 100644 python/ql/src/semmle/python/security/dataflow/SqlInjectionCustomizations.qll

diff --git a/python/ql/src/Security/CWE-089/SqlInjection.ql b/python/ql/src/Security/CWE-089/SqlInjection.ql
index 1d34b1a58f2..9e6cf08899d 100644
--- a/python/ql/src/Security/CWE-089/SqlInjection.ql
+++ b/python/ql/src/Security/CWE-089/SqlInjection.ql
@@ -16,7 +16,7 @@ import python
 import semmle.python.security.dataflow.SqlInjection
 import DataFlow::PathGraph
 
-from SQLInjectionConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
+from SqlInjection::Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
 where config.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "This SQL query depends on $@.", source.getNode(),
   "a user-provided value"
diff --git a/python/ql/src/semmle/python/security/dataflow/SqlInjection.qll b/python/ql/src/semmle/python/security/dataflow/SqlInjection.qll
index 6ecb2438c23..3195962e62e 100644
--- a/python/ql/src/semmle/python/security/dataflow/SqlInjection.qll
+++ b/python/ql/src/semmle/python/security/dataflow/SqlInjection.qll
@@ -1,26 +1,42 @@
 /**
- * Provides a taint-tracking configuration for detecting SQL injection
- * vulnerabilities.
+ * Provides a taint-tracking configuration for detecting "SQL Injection" vulnerabilities.
+ *
+ * Note, for performance reasons: only import this file if
+ * `SqlInjection::Configuration` is needed, otherwise
+ * `SqlInjectionCustomizations` should be imported instead.
  */
 
-import python
+private import python
 import semmle.python.dataflow.new.DataFlow
 import semmle.python.dataflow.new.TaintTracking
-import semmle.python.Concepts
-import semmle.python.dataflow.new.RemoteFlowSources
-import semmle.python.dataflow.new.BarrierGuards
 
 /**
- * A taint-tracking configuration for detecting SQL injection vulnerabilities.
+ * Provides a taint-tracking configuration for detecting "SQL Injection" vulnerabilities.
  */
-class SQLInjectionConfiguration extends TaintTracking::Configuration {
-  SQLInjectionConfiguration() { this = "SQLInjectionConfiguration" }
+module SqlInjection {
+  import SqlInjectionCustomizations::SqlInjection
 
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+  /**
+   * A taint-tracking configuration for detecting "SQL Injection" vulnerabilities.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "SqlInjection" }
 
-  override predicate isSink(DataFlow::Node sink) { sink = any(SqlExecution e).getSql() }
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
 
-  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
-    guard instanceof StringConstCompare
+    override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+    override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+
+    override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+      guard instanceof SanitizerGuard
+    }
   }
 }
+
+/**
+ * DEPRECATED: Don't extend this class for customization, since this will lead to bad
+ * performance, instead use the new `SqlInjectionCustomizations.qll` file, and extend
+ * its' classes.
+ */
+deprecated class SQLInjectionConfiguration = SqlInjection::Configuration;
diff --git a/python/ql/src/semmle/python/security/dataflow/SqlInjectionCustomizations.qll b/python/ql/src/semmle/python/security/dataflow/SqlInjectionCustomizations.qll
new file mode 100644
index 00000000000..a1acbafd29d
--- /dev/null
+++ b/python/ql/src/semmle/python/security/dataflow/SqlInjectionCustomizations.qll
@@ -0,0 +1,55 @@
+/**
+ * Provides default sources, sinks and sanitizers for detecting
+ * "SQL Injection"
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+
+private import python
+private import semmle.python.dataflow.new.DataFlow
+private import semmle.python.Concepts
+private import semmle.python.dataflow.new.RemoteFlowSources
+private import semmle.python.dataflow.new.BarrierGuards
+
+/**
+ * Provides default sources, sinks and sanitizers for detecting
+ * "SQL Injection"
+ * vulnerabilities, as well as extension points for adding your own.
+ */
+module SqlInjection {
+  /**
+   * A data flow source for "SQL Injection" vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for "SQL Injection" vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for "SQL Injection" vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A sanitizer guard for "SQL Injection" vulnerabilities.
+   */
+  abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
+
+  /**
+   * A source of remote user input, considered as a flow source.
+   */
+  class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
+
+  /**
+   * A SQL statement of a SQL execution, considered as a flow sink.
+   */
+  class SqlExecutionAsSink extends Sink {
+    SqlExecutionAsSink() { this = any(SqlExecution e).getSql() }
+  }
+
+  /**
+   * A comparison with a constant string, considered as a sanitizer-guard.
+   */
+  class StringConstCompareAsSanitizerGuard extends SanitizerGuard, StringConstCompare { }
+}