Skip to content

Commit 6cf9968

Browse files
MathiasVPMathiasVP
authored
Merge pull request #13985 from alexet/ir-tainted-sql
CPP: Convert SQL tainted away from away from DefaultTaintTracking.
2 parents 89b790d + 949b0a2 commit 6cf9968

File tree

2 files changed

+53
-37
lines changed

2 files changed

+53
-37
lines changed

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

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,41 +14,55 @@
1414

1515
import cpp
1616
import semmle.code.cpp.security.Security
17+
import semmle.code.cpp.security.FlowSources
1718
import semmle.code.cpp.security.FunctionWithWrappers
18-
import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
19-
import TaintedWithPath
19+
import semmle.code.cpp.ir.IR
20+
import semmle.code.cpp.ir.dataflow.TaintTracking
21+
import SqlTainted::PathGraph
2022

2123
class SqlLikeFunction extends FunctionWithWrappers {
2224
SqlLikeFunction() { sqlArgument(this.getName(), _) }
2325

2426
override predicate interestingArg(int arg) { sqlArgument(this.getName(), arg) }
2527
}
2628

27-
class Configuration extends TaintTrackingConfiguration {
28-
override predicate isSink(Element tainted) {
29-
exists(SqlLikeFunction runSql | runSql.outermostWrapperFunctionCall(tainted, _))
29+
Expr asSinkExpr(DataFlow::Node node) {
30+
result = node.asIndirectArgument()
31+
or
32+
// We want the conversion so we only get one node for the expression
33+
result = node.asConvertedExpr()
34+
}
35+
36+
module SqlTaintedConfig implements DataFlow::ConfigSig {
37+
predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
38+
39+
predicate isSink(DataFlow::Node node) {
40+
exists(SqlLikeFunction runSql | runSql.outermostWrapperFunctionCall(asSinkExpr(node), _))
3041
}
3142

32-
override predicate isBarrier(Expr e) {
33-
super.isBarrier(e)
34-
or
35-
e.getUnspecifiedType() instanceof IntegralType
36-
or
43+
predicate isBarrier(DataFlow::Node node) {
44+
node.asExpr().getUnspecifiedType() instanceof IntegralType
45+
}
46+
47+
predicate isBarrierIn(DataFlow::Node node) {
3748
exists(SqlBarrierFunction sql, int arg, FunctionInput input |
38-
e = sql.getACallToThisFunction().getArgument(arg) and
49+
node.asIndirectArgument() = sql.getACallToThisFunction().getArgument(arg) and
3950
input.isParameterDeref(arg) and
4051
sql.barrierSqlArgument(input, _)
4152
)
4253
}
4354
}
4455

56+
module SqlTainted = TaintTracking::Global<SqlTaintedConfig>;
57+
4558
from
46-
SqlLikeFunction runSql, Expr taintedArg, Expr taintSource, PathNode sourceNode, PathNode sinkNode,
47-
string taintCause, string callChain
59+
SqlLikeFunction runSql, Expr taintedArg, FlowSource taintSource, SqlTainted::PathNode sourceNode,
60+
SqlTainted::PathNode sinkNode, string callChain
4861
where
4962
runSql.outermostWrapperFunctionCall(taintedArg, callChain) and
50-
taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
51-
isUserInput(taintSource, taintCause)
63+
SqlTainted::flowPath(sourceNode, sinkNode) and
64+
taintedArg = asSinkExpr(sinkNode.getNode()) and
65+
taintSource = sourceNode.getNode()
5266
select taintedArg, sourceNode, sinkNode,
5367
"This argument to a SQL query function is derived from $@ and then passed to " + callChain + ".",
54-
taintSource, "user input (" + taintCause + ")"
68+
taintSource, "user input (" + taintSource.getSourceType() + ")"

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

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,27 @@
11
edges
2-
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 |
3-
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 |
4-
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 |
5-
| test.c:15:20:15:23 | argv | test.c:21:18:21:23 | query1 |
6-
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
7-
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
8-
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
9-
| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array |
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-
subpaths
2+
| test.c:14:27:14:30 | argv | test.c:21:18:21:23 | query1 indirection |
3+
| test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection |
4+
| test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection |
5+
| test.cpp:39:27:39:30 | argv | test.cpp:43:27:43:33 | access to array |
6+
| test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array |
7+
| test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection |
8+
| test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection |
139
nodes
14-
| test.c:15:20:15:23 | argv | semmle.label | argv |
15-
| test.c:15:20:15:23 | argv | semmle.label | argv |
16-
| test.c:21:18:21:23 | query1 | semmle.label | query1 |
17-
| test.c:21:18:21:23 | query1 | semmle.label | query1 |
18-
| test.cpp:43:27:43:30 | argv | semmle.label | argv |
19-
| test.cpp:43:27:43:30 | argv | semmle.label | argv |
20-
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
21-
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
10+
| test.c:14:27:14:30 | argv | semmle.label | argv |
11+
| test.c:14:27:14:30 | argv indirection | semmle.label | argv indirection |
12+
| test.c:14:27:14:30 | argv indirection | semmle.label | argv indirection |
13+
| test.c:21:18:21:23 | query1 indirection | semmle.label | query1 indirection |
14+
| test.cpp:39:27:39:30 | argv | semmle.label | argv |
15+
| test.cpp:39:27:39:30 | argv indirection | semmle.label | argv indirection |
16+
| test.cpp:39:27:39:30 | argv indirection | semmle.label | argv indirection |
2217
| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
18+
| test.cpp:43:27:43:33 | access to array indirection | semmle.label | access to array indirection |
19+
subpaths
2320
#select
24-
| 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) |
25-
| 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) |
21+
| test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | argv | test.c:21:18:21:23 | query1 indirection | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | argv | user input (a command-line argument) |
22+
| test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | argv indirection | user input (a command-line argument) |
23+
| test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | argv indirection | user input (a command-line argument) |
24+
| test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39: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:39:27:39:30 | argv | user input (a command-line argument) |
25+
| test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39:30 | argv indirection | 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:39:27:39:30 | argv indirection | user input (a command-line argument) |
26+
| test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)). | test.cpp:39:27:39:30 | argv indirection | user input (a command-line argument) |
27+
| test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)). | test.cpp:39:27:39:30 | argv indirection | user input (a command-line argument) |

0 commit comments

Comments
 (0)