hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 11:15:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
C++: Promote `cpp/sql-injection-via-pqxx` out of experimental
This commit is contained in:
Robert Marsh
2021-09-21 10:13:57 -07:00
committed by GitHub
parent 97c2917c16 478093aa89
commit d62f76afa6
13 changed files with 226 additions and 180 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cpp/change-notes/2021-06-22-sql-tainted.md Normal file
View File

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

3
cpp/ql/lib/semmle/code/cpp/models/Models.qll
View File

@@ -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

8
cpp/ql/lib/semmle/code/cpp/models/implementations/MySql.qll Normal file
View File

@@ -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) }
}

94
cpp/ql/lib/semmle/code/cpp/models/implementations/PostgreSql.qll Normal file
View File

@@ -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)
)
}
}

8
cpp/ql/lib/semmle/code/cpp/models/implementations/SqLite3.qll Normal file
View File

@@ -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) }
}

30
cpp/ql/lib/semmle/code/cpp/models/interfaces/Sql.qll Normal file
View File

@@ -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);
}

13
cpp/ql/lib/semmle/code/cpp/security/Security.qll
View File

@@ -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)
)
}
/**

10
cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql
View File

@@ -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, _)
)
}
}

28
cpp/ql/src/experimental/Security/CWE/CWE-089/SqlPqxxTainted.cpp
View File

@@ -1,28 +0,0 @@
#include <iostream>
#include <stdexcept>
#include <pqxx/pqxx>
int main(int argc, char ** argv) {
if (argc != 2) {
throw std::runtime_error("Give me a string!");
}
pqxx::connection c;
pqxx::work w(c);
// BAD
char *userName = argv[1];
char query1[1000] = {0};
sprintf(query1, "SELECT UID FROM USERS where name = \"%s\"", userName);
pqxx::row r = w.exec1(query1);
w.commit();
std::cout << r[0].as<int>() << std::endl;
// GOOD
pqxx::result r2 = w.exec("SELECT " + w.quote(argv[1]));
w.commit();
std::cout << r2[0][0].c_str() << std::endl;
return 0;
}

31
cpp/ql/src/experimental/Security/CWE/CWE-089/SqlPqxxTainted.qhelp
View File

@@ -1,31 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>The code passes user input as part of a SQL query without escaping special elements.
It generates a SQL query to Postgres using <code>sprintf</code>,
with the user-supplied data directly passed as an argument
to <code>sprintf</code>. This leaves the code vulnerable to attack by SQL Injection.</p>
</overview>
<recommendation>
<p>Use a library routine to escape characters in the user-supplied
string before converting it to SQL. Use <code>esc</code> and <code>quote</code> pqxx library functions.</p>
</recommendation>
<example>
<sample src="SqlPqxxTainted.cpp" />
</example>
<references>
<li>MSDN Library: <a href="https://docs.microsoft.com/en-us/sql/relational-databases/security/sql-injection">SQL Injection</a>.</li>
<!-- LocalWords: SQL CWE
-->
</references>
</qhelp>

113
cpp/ql/src/experimental/Security/CWE/CWE-089/SqlPqxxTainted.ql
View File

@@ -1,113 +0,0 @@
/**
* @name Uncontrolled data in SQL query to Postgres
* @description Including user-supplied data in a SQL query to Postgres
* without neutralizing special elements can make code
* vulnerable to SQL Injection.
* @kind path-problem
* @problem.severity error
* @precision high
* @id cpp/sql-injection-via-pqxx
* @tags security
* external/cwe/cwe-089
*/
import cpp
import semmle.code.cpp.security.Security
import semmle.code.cpp.dataflow.TaintTracking
import DataFlow::PathGraph
predicate pqxxTransationClassNames(string className, string namespace) {
namespace = "pqxx" and
className in [
"dbtransaction", "nontransaction", "basic_robusttransaction", "robusttransaction",
"subtransaction", "transaction", "basic_transaction", "transaction_base", "work"
]
}
predicate pqxxConnectionClassNames(string className, string namespace) {
namespace = "pqxx" and
className in ["connection_base", "basic_connection", "connection"]
}
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
}
predicate pqxxConnectionSqlArgument(string function, int arg) { function = "prepare" and arg = 1 }
Expr getPqxxSqlArgument() {
exists(FunctionCall fc, Expr e, int argIndex, UserType t |
// examples: 'work' for 'work.exec(...)'; '->' for 'tx->exec()'.
e = fc.getQualifier() and
// to find ConnectionHandle/TransationHandle and similar classes which override '->' operator behavior
// and return pointer to a connection/transation object
e.getType().refersTo(t) and
// transaction exec and connection prepare variations
(
pqxxTransationClassNames(t.getName(), t.getNamespace().getName()) and
pqxxTransactionSqlArgument(fc.getTarget().getName(), argIndex)
or
pqxxConnectionClassNames(t.getName(), t.getNamespace().getName()) and
pqxxConnectionSqlArgument(fc.getTarget().getName(), argIndex)
) and
result = fc.getArgument(argIndex)
)
}
predicate pqxxEscapeArgument(string function, int arg) {
arg = 0 and
function in ["esc", "esc_raw", "quote", "quote_raw", "quote_name", "quote_table", "esc_like"]
}
predicate isEscapedPqxxArgument(Expr argExpr) {
exists(FunctionCall fc, Expr e, int argIndex, UserType t |
// examples: 'work' for 'work.exec(...)'; '->' for 'tx->exec()'.
e = fc.getQualifier() and
// to find ConnectionHandle/TransationHandle and similar classes which override '->' operator behavior
// and return pointer to a connection/transation object
e.getType().refersTo(t) and
// transaction and connection escape functions
(
pqxxTransationClassNames(t.getName(), t.getNamespace().getName()) or
pqxxConnectionClassNames(t.getName(), t.getNamespace().getName())
) and
pqxxEscapeArgument(fc.getTarget().getName(), argIndex) and
// is escaped arg == argExpr
argExpr = fc.getArgument(argIndex)
)
}
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "SqlPqxxTainted" }
override predicate isSource(DataFlow::Node source) { isUserInput(source.asExpr(), _) }
override predicate isSink(DataFlow::Node sink) { sink.asExpr() = getPqxxSqlArgument() }
override predicate isSanitizer(DataFlow::Node node) { isEscapedPqxxArgument(node.asExpr()) }
}
from DataFlow::PathNode source, DataFlow::PathNode sink, Configuration config, string taintCause
where
config.hasFlowPath(source, sink) and
isUserInput(source.getNode().asExpr(), taintCause)
select sink, source, sink, "This argument to a SQL query function is derived from $@", source,
"user input (" + taintCause + ")"

18
cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected
View File

@@ -5,6 +5,14 @@ edges
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 |
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 indirection |
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 indirection |
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | (const char *)... |
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | (const char *)... |
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array indirection |
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array indirection |
nodes
| test.c:15:20:15:23 | argv | semmle.label | argv |
| test.c:15:20:15:23 | argv | semmle.label | argv |
@@ -13,5 +21,15 @@ nodes
| test.c:21:18:21:23 | query1 | semmle.label | query1 |
| test.c:21:18:21:23 | query1 indirection | semmle.label | query1 indirection |
| test.c:21:18:21:23 | query1 indirection | semmle.label | query1 indirection |
| test.cpp:43:27:43:30 | argv | semmle.label | argv |
| test.cpp:43:27:43:30 | argv | semmle.label | argv |
| test.cpp:43:27:43:33 | (const char *)... | semmle.label | (const char *)... |
| test.cpp:43:27:43:33 | (const char *)... | semmle.label | (const char *)... |
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
| test.cpp:43:27:43:33 | access to array indirection | semmle.label | access to array indirection |
| test.cpp:43:27:43:33 | access to array indirection | semmle.label | access to array indirection |
#select
| test.c:21:18:21:23 | query1 | test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg) | test.c:15:20:15:23 | argv | user input (argv) |
| test.cpp:43:27:43:33 | access to array | test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)) | test.cpp:43:27:43:30 | argv | user input (argv) |

48
cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/test.cpp Normal file
View File

@@ -0,0 +1,48 @@
int sprintf(char* str, const char* format, ...);
namespace std
{
template<class charT> struct char_traits;
template <class T> class allocator {
public:
allocator() throw();
};
template<class charT, class traits = char_traits<charT>, class Allocator = allocator<charT> >
class basic_string {
public:
explicit basic_string(const Allocator& a = Allocator());
basic_string(const charT* s, const Allocator& a = Allocator());
const charT* c_str() const;
};
typedef basic_string<char> string;
}
namespace pqxx {
struct connection {};
struct row {};
struct result {};
struct work {
work(connection&);
row exec1(const char*);
result exec(const std::string&);
std::string quote(const char*);
};
}
int main(int argc, char** argv) {
pqxx::connection c;
pqxx::work w(c);
pqxx::row r = w.exec1(argv[1]); // BAD
pqxx::result r2 = w.exec(w.quote(argv[1])); // GOOD
return 0;
}