Skip to content

Commit 90633b9

Browse files
MathiasVPMathiasVP
committed
C++: Make the new SQL abstract classes extend 'Function' instead. This is more in line with how we model RemoteFlowFunction.
1 parent 90fe5c5 commit 90633b9

File tree

9 files changed

+167
-166
lines changed

9 files changed

+167
-166
lines changed

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import cpp
1616
import semmle.code.cpp.security.Security
1717
import semmle.code.cpp.security.FunctionWithWrappers
1818
import semmle.code.cpp.security.TaintTracking
19-
import semmle.code.cpp.security.Sql
2019
import TaintedWithPath
2120

2221
class SQLLikeFunction extends FunctionWithWrappers {
@@ -35,10 +34,10 @@ class Configuration extends TaintTrackingConfiguration {
3534
or
3635
e.getUnspecifiedType() instanceof IntegralType
3736
or
38-
exists(SqlFunctionality sql, int arg, Function func, FunctionInput input |
39-
e = func.getACallToThisFunction().getArgument(arg) and
37+
exists(SqlBarrier sqlFunc, int arg, FunctionInput input |
38+
e = sqlFunc.getACallToThisFunction().getArgument(arg) and
4039
input.isParameterDeref(arg) and
41-
sql.getAnEscapedParameter(func, input, _)
40+
sqlFunc.getAnEscapedParameter(input, _)
4241
)
4342
}
4443
}

‎cpp/ql/src/semmle/code/cpp/models/Models.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,6 @@ private import implementations.Recv
3333
private import implementations.Accept
3434
private import implementations.Poll
3535
private import implementations.Select
36+
private import implementations.MySql
37+
private import implementations.SqLite3
38+
private import implementations.PostgreSql

‎cpp/ql/src/semmle/code/cpp/models/implementations/MySql.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
private import semmle.code.cpp.models.interfaces.Sql
2+
private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs
3+
4+
private class MySqlSink extends SqlSink {
5+
MySqlSink() { this.hasName(["mysql_query", "mysql_real_query"]) }
6+
7+
override predicate getAnSqlParameter(FunctionInput input) { input.isParameterDeref(1) }
8+
}

‎cpp/ql/src/semmle/code/cpp/models/implementations/PostgreSql.qll

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
private import semmle.code.cpp.models.interfaces.Sql
2+
private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs
3+
4+
private predicate pqxxTransactionSqlArgument(string function, int arg) {
5+
function = "exec" and arg = 0
6+
or
7+
function = "exec0" and arg = 0
8+
or
9+
function = "exec1" and arg = 0
10+
or
11+
function = "exec_n" and arg = 1
12+
or
13+
function = "exec_params" and arg = 0
14+
or
15+
function = "exec_params0" and arg = 0
16+
or
17+
function = "exec_params1" and arg = 0
18+
or
19+
function = "exec_params_n" and arg = 1
20+
or
21+
function = "query_value" and arg = 0
22+
or
23+
function = "stream" and arg = 0
24+
}
25+
26+
private predicate pqxxConnectionSqlArgument(string function, int arg) {
27+
function = "prepare" and arg = 1
28+
}
29+
30+
private predicate pqxxTransationClassNames(string className, string namespace) {
31+
namespace = "pqxx" and
32+
className in [
33+
"dbtransaction", "nontransaction", "basic_robusttransaction", "robusttransaction",
34+
"subtransaction", "transaction", "basic_transaction", "transaction_base", "work"
35+
]
36+
}
37+
38+
private predicate pqxxConnectionClassNames(string className, string namespace) {
39+
namespace = "pqxx" and
40+
className in ["connection_base", "basic_connection", "connection"]
41+
}
42+
43+
private predicate pqxxEscapeArgument(string function, int arg) {
44+
arg = 0 and
45+
function in ["esc", "esc_raw", "quote", "quote_raw", "quote_name", "quote_table", "esc_like"]
46+
}
47+
48+
private class PostgreSqlSink extends SqlSink {
49+
PostgreSqlSink() {
50+
exists(Class c |
51+
this.getDeclaringType() = c and
52+
// transaction exec and connection prepare variations
53+
(
54+
pqxxTransationClassNames(c.getName(), c.getNamespace().getName()) and
55+
pqxxTransactionSqlArgument(this.getName(), _)
56+
or
57+
pqxxConnectionSqlArgument(this.getName(), _) and
58+
pqxxConnectionClassNames(c.getName(), c.getNamespace().getName())
59+
)
60+
)
61+
}
62+
63+
override predicate getAnSqlParameter(FunctionInput input) {
64+
exists(int argIndex |
65+
pqxxTransactionSqlArgument(this.getName(), argIndex)
66+
or
67+
pqxxConnectionSqlArgument(this.getName(), argIndex)
68+
|
69+
input.isParameterDeref(argIndex)
70+
)
71+
}
72+
}
73+
74+
private class PostgreSqlBarrier extends SqlBarrier {
75+
PostgreSqlBarrier() {
76+
exists(Class c |
77+
this.getDeclaringType() = c and
78+
// transaction and connection escape functions
79+
(
80+
pqxxTransationClassNames(c.getName(), c.getNamespace().getName()) or
81+
pqxxConnectionClassNames(c.getName(), c.getNamespace().getName())
82+
) and
83+
pqxxEscapeArgument(this.getName(), _)
84+
)
85+
}
86+
87+
override predicate getAnEscapedParameter(FunctionInput input, FunctionOutput output) {
88+
exists(int argIndex |
89+
input.isParameterDeref(argIndex) and
90+
output.isReturnValueDeref()
91+
)
92+
}
93+
}

‎cpp/ql/src/semmle/code/cpp/models/implementations/SqLite3.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
private import semmle.code.cpp.models.interfaces.Sql
2+
private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs
3+
4+
private class SqLite3Sink extends SqlSink {
5+
SqLite3Sink() { this.hasName("sqlite3_exec") }
6+
7+
override predicate getAnSqlParameter(FunctionInput input) { input.isParameterDeref(1) }
8+
}

‎cpp/ql/src/semmle/code/cpp/models/interfaces/Sql.qll

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Provides abstract classes for modeling functions that execute and escape SQL query strings.
3+
* To use this QL library, create a QL class extending `SqlSink` or `SqlBarrier` with a
4+
* characteristic predicate that selects the function or set of functions you are modeling.
5+
* Within that class, override the predicates provided by the class to match the way a
6+
* parameter flows into the function and, in the case of `SqlBarrier`, out of the function.
7+
*/
8+
9+
private import cpp
10+
11+
/**
12+
* An abstract class that represents a function that executes an SQL query.
13+
*/
14+
abstract class SqlSink extends Function {
15+
/**
16+
* Holds if `input` to this function represents data that is passed to an SQL server.
17+
*/
18+
abstract predicate getAnSqlParameter(FunctionInput input);
19+
}
20+
21+
/**
22+
* An abstract class that represents a function that escapes an SQL query string.
23+
*/
24+
abstract class SqlBarrier extends Function {
25+
/**
26+
* Holds if the `output` escapes the SQL input `input` such that is it safe to pass to
27+
* an `SqlSink`.
28+
*/
29+
abstract predicate getAnEscapedParameter(FunctionInput input, FunctionOutput output);
30+
}

‎cpp/ql/src/semmle/code/cpp/security/Security.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import semmle.code.cpp.exprs.Expr
77
import semmle.code.cpp.commons.Environment
88
import semmle.code.cpp.security.SecurityOptions
99
import semmle.code.cpp.models.interfaces.FlowSource
10-
private import Sql
10+
import semmle.code.cpp.models.interfaces.Sql
1111

1212
/**
1313
* Extend this class to customize the security queries for
@@ -35,10 +35,10 @@ class SecurityOptions extends string {
3535
* An argument to a function that is passed to a SQL server.
3636
*/
3737
predicate sqlArgument(string function, int arg) {
38-
exists(Function func, FunctionInput input, SqlFunctionality sql |
39-
func.hasName(function) and
38+
exists(FunctionInput input, SqlSink sqlSink |
39+
sqlSink.hasName(function) and
4040
input.isParameterDeref(arg) and
41-
sql.getAnSqlParameter(func, input)
41+
sqlSink.getAnSqlParameter(input)
4242
)
4343
}
4444

‎cpp/ql/src/semmle/code/cpp/security/Sql.qll

Lines changed: 0 additions & 140 deletions
This file was deleted.

‎cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ edges
55
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 |
66
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 indirection |
77
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 indirection |
8-
| test.cpp:44:27:44:30 | argv | test.cpp:44:27:44:33 | (const char *)... |
9-
| test.cpp:44:27:44:30 | argv | test.cpp:44:27:44:33 | (const char *)... |
10-
| test.cpp:44:27:44:30 | argv | test.cpp:44:27:44:33 | access to array |
11-
| test.cpp:44:27:44:30 | argv | test.cpp:44:27:44:33 | access to array |
12-
| test.cpp:44:27:44:30 | argv | test.cpp:44:27:44:33 | access to array |
13-
| test.cpp:44:27:44:30 | argv | test.cpp:44:27:44:33 | access to array |
14-
| test.cpp:44:27:44:30 | argv | test.cpp:44:27:44:33 | access to array indirection |
15-
| test.cpp:44:27:44:30 | argv | test.cpp:44:27:44:33 | access to array indirection |
8+
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | (const char *)... |
9+
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | (const char *)... |
10+
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
11+
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
12+
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
13+
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
14+
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array indirection |
15+
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array indirection |
1616
nodes
1717
| test.c:15:20:15:23 | argv | semmle.label | argv |
1818
| test.c:15:20:15:23 | argv | semmle.label | argv |
@@ -21,15 +21,15 @@ nodes
2121
| test.c:21:18:21:23 | query1 | semmle.label | query1 |
2222
| test.c:21:18:21:23 | query1 indirection | semmle.label | query1 indirection |
2323
| test.c:21:18:21:23 | query1 indirection | semmle.label | query1 indirection |
24-
| test.cpp:44:27:44:30 | argv | semmle.label | argv |
25-
| test.cpp:44:27:44:30 | argv | semmle.label | argv |
26-
| test.cpp:44:27:44:33 | (const char *)... | semmle.label | (const char *)... |
27-
| test.cpp:44:27:44:33 | (const char *)... | semmle.label | (const char *)... |
28-
| test.cpp:44:27:44:33 | access to array | semmle.label | access to array |
29-
| test.cpp:44:27:44:33 | access to array | semmle.label | access to array |
30-
| test.cpp:44:27:44:33 | access to array | semmle.label | access to array |
31-
| test.cpp:44:27:44:33 | access to array indirection | semmle.label | access to array indirection |
32-
| test.cpp:44:27:44:33 | access to array indirection | semmle.label | access to array indirection |
24+
| test.cpp:43:27:43:30 | argv | semmle.label | argv |
25+
| test.cpp:43:27:43:30 | argv | semmle.label | argv |
26+
| test.cpp:43:27:43:33 | (const char *)... | semmle.label | (const char *)... |
27+
| test.cpp:43:27:43:33 | (const char *)... | semmle.label | (const char *)... |
28+
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
29+
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
30+
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
31+
| test.cpp:43:27:43:33 | access to array indirection | semmle.label | access to array indirection |
32+
| test.cpp:43:27:43:33 | access to array indirection | semmle.label | access to array indirection |
3333
#select
3434
| 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) |
35-
| test.cpp:44:27:44:33 | access to array | test.cpp:44:27:44:30 | argv | test.cpp:44:27:44: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:44:27:44:30 | argv | user input (argv) |
35+
| 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) |

0 commit comments

Comments
 (0)