Merge pull request #6133 from MathiasVP/promote-sql-pqxx

C++: Promote `cpp/sql-injection-via-pqxx` out of experimental

Author: rdmarsh2

cpp/change-notes/2021-06-22-sql-tainted.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The 'Uncontrolled data in SQL query' (cpp/sql-injection) query now supports the `libpqxx` library.

cpp/ql/lib/semmle/code/cpp/models/Models.qll

@@ -33,3 +33,6 @@ private import implementations.Recv
 private import implementations.Accept
 private import implementations.Poll
 private import implementations.Select
+private import implementations.MySql
+private import implementations.SqLite3
+private import implementations.PostgreSql

cpp/ql/lib/semmle/code/cpp/models/implementations/MySql.qll

@@ -0,0 +1,8 @@
+private import semmle.code.cpp.models.interfaces.Sql
+private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs
+
+private class MySqlExecutionFunction extends SqlExecutionFunction {
+  MySqlExecutionFunction() { this.hasName(["mysql_query", "mysql_real_query"]) }
+
+  override predicate hasSqlArgument(FunctionInput input) { input.isParameterDeref(1) }
+}

cpp/ql/lib/semmle/code/cpp/models/implementations/PostgreSql.qll

@@ -0,0 +1,94 @@
+private import semmle.code.cpp.models.interfaces.Sql
+private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs
+
+private predicate pqxxTransactionSqlArgument(string function, int arg) {
+  function = "exec" and arg = 0
+  or
+  function = "exec0" and arg = 0
+  or
+  function = "exec1" and arg = 0
+  or
+  function = "exec_n" and arg = 1
+  or
+  function = "exec_params" and arg = 0
+  or
+  function = "exec_params0" and arg = 0
+  or
+  function = "exec_params1" and arg = 0
+  or
+  function = "exec_params_n" and arg = 1
+  or
+  function = "query_value" and arg = 0
+  or
+  function = "stream" and arg = 0
+}
+
+private predicate pqxxConnectionSqlArgument(string function, int arg) {
+  function = "prepare" and arg = 1
+}
+
+private predicate pqxxTransationClassNames(string className, string namespace) {
+  namespace = "pqxx" and
+  className in [
+      "dbtransaction", "nontransaction", "basic_robusttransaction", "robusttransaction",
+      "subtransaction", "transaction", "basic_transaction", "transaction_base", "work"
+    ]
+}
+
+private predicate pqxxConnectionClassNames(string className, string namespace) {
+  namespace = "pqxx" and
+  className in ["connection_base", "basic_connection", "connection"]
+}
+
+private predicate pqxxEscapeArgument(string function, int arg) {
+  arg = 0 and
+  function in ["esc", "esc_raw", "quote", "quote_raw", "quote_name", "quote_table", "esc_like"]
+}
+
+private class PostgreSqlExecutionFunction extends SqlExecutionFunction {
+  PostgreSqlExecutionFunction() {
+    exists(Class c |
+      this.getDeclaringType() = c and
+      // transaction exec and connection prepare variations
+      (
+        pqxxTransationClassNames(c.getName(), c.getNamespace().getName()) and
+        pqxxTransactionSqlArgument(this.getName(), _)
+        or
+        pqxxConnectionSqlArgument(this.getName(), _) and
+        pqxxConnectionClassNames(c.getName(), c.getNamespace().getName())
+      )
+    )
+  }
+
+  override predicate hasSqlArgument(FunctionInput input) {
+    exists(int argIndex |
+      pqxxTransactionSqlArgument(this.getName(), argIndex)
+      or
+      pqxxConnectionSqlArgument(this.getName(), argIndex)
+    |
+      input.isParameterDeref(argIndex)
+    )
+  }
+}
+
+private class PostgreSqlBarrierFunction extends SqlBarrierFunction {
+  PostgreSqlBarrierFunction() {
+    exists(Class c |
+      this.getDeclaringType() = c and
+      // transaction and connection escape functions
+      (
+        pqxxTransationClassNames(c.getName(), c.getNamespace().getName()) or
+        pqxxConnectionClassNames(c.getName(), c.getNamespace().getName())
+      ) and
+      pqxxEscapeArgument(this.getName(), _)
+    )
+  }
+
+  override predicate barrierSqlArgument(FunctionInput input, FunctionOutput output) {
+    exists(int argIndex |
+      input.isParameterDeref(argIndex) and
+      output.isReturnValueDeref() and
+      pqxxEscapeArgument(this.getName(), argIndex)
+    )
+  }
+}

cpp/ql/lib/semmle/code/cpp/models/implementations/SqLite3.qll

@@ -0,0 +1,8 @@
+private import semmle.code.cpp.models.interfaces.Sql
+private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs
+
+private class SqLite3ExecutionFunction extends SqlExecutionFunction {
+  SqLite3ExecutionFunction() { this.hasName("sqlite3_exec") }
+
+  override predicate hasSqlArgument(FunctionInput input) { input.isParameterDeref(1) }
+}

cpp/ql/lib/semmle/code/cpp/models/interfaces/Sql.qll

@@ -0,0 +1,30 @@
+/**
+ * Provides abstract classes for modeling functions that execute and escape SQL query strings.
+ * To extend this QL library, create a QL class extending `SqlExecutionFunction` or `SqlEscapeFunction`
+ * with a characteristic predicate that selects the function or set of functions you are modeling.
+ * Within that class, override the predicates provided by the class to match the way a
+ * parameter flows into the function and, in the case of `SqlEscapeFunction`, out of the function.
+ */
+
+private import cpp
+
+/**
+ * An abstract class that represents a function that executes an SQL query.
+ */
+abstract class SqlExecutionFunction extends Function {
+  /**
+   * Holds if `input` to this function represents SQL code to be executed.
+   */
+  abstract predicate hasSqlArgument(FunctionInput input);
+}
+
+/**
+ * An abstract class that represents a function that is a barrier to an SQL query string.
+ */
+abstract class SqlBarrierFunction extends Function {
+  /**
+   * Holds if the `output` is a barrier to the SQL input `input` such that is it safe to pass to
+   * an `SqlExecutionFunction`.
+   */
+  abstract predicate barrierSqlArgument(FunctionInput input, FunctionOutput output);
+}

cpp/ql/lib/semmle/code/cpp/security/Security.qll

@@ -7,6 +7,7 @@ import semmle.code.cpp.exprs.Expr
 import semmle.code.cpp.commons.Environment
 import semmle.code.cpp.security.SecurityOptions
 import semmle.code.cpp.models.interfaces.FlowSource
+import semmle.code.cpp.models.interfaces.Sql
 
 /**
  * Extend this class to customize the security queries for
@@ -34,13 +35,11 @@ class SecurityOptions extends string {
    * An argument to a function that is passed to a SQL server.
    */
   predicate sqlArgument(string function, int arg) {
-    // MySQL C API
-    function = "mysql_query" and arg = 1
-    or
-    function = "mysql_real_query" and arg = 1
-    or
-    // SQLite3 C API
-    function = "sqlite3_exec" and arg = 1
+    exists(FunctionInput input, SqlExecutionFunction sql |
+      sql.hasName(function) and
+      input.isParameterDeref(arg) and
+      sql.hasSqlArgument(input)
+    )
   }
 
   /**

cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql

@@ -30,7 +30,15 @@ class Configuration extends TaintTrackingConfiguration {
   }
 
   override predicate isBarrier(Expr e) {
-    super.isBarrier(e) or e.getUnspecifiedType() instanceof IntegralType
+    super.isBarrier(e)
+    or
+    e.getUnspecifiedType() instanceof IntegralType
+    or
+    exists(SqlBarrierFunction sql, int arg, FunctionInput input |
+      e = sql.getACallToThisFunction().getArgument(arg) and
+      input.isParameterDeref(arg) and
+      sql.barrierSqlArgument(input, _)
+    )
   }
 }