diff --git a/cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql b/cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql index f3ff16e8f7b7c..d83b92fbf6489 100644 --- a/cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql +++ b/cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql @@ -14,9 +14,10 @@ import cpp import semmle.code.cpp.security.Security +import semmle.code.cpp.security.FlowSources import semmle.code.cpp.security.FunctionWithWrappers -import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl -import TaintedWithPath +import semmle.code.cpp.ir.IR +import semmle.code.cpp.ir.dataflow.TaintTracking class SqlLikeFunction extends FunctionWithWrappers { SqlLikeFunction() { sqlArgument(this.getName(), _) } @@ -24,31 +25,45 @@ class SqlLikeFunction extends FunctionWithWrappers { override predicate interestingArg(int arg) { sqlArgument(this.getName(), arg) } } -class Configuration extends TaintTrackingConfiguration { - override predicate isSink(Element tainted) { - exists(SqlLikeFunction runSql | runSql.outermostWrapperFunctionCall(tainted, _)) +Expr asSinkExpr(DataFlow::Node node) { + result = node.asIndirectArgument() + or + // We want the conversion so we only get one node for the expression + result = node.asConvertedExpr() +} + +module SqlTaintedConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node instanceof FlowSource } + + predicate isSink(DataFlow::Node node) { + exists(SqlLikeFunction runSql | runSql.outermostWrapperFunctionCall(asSinkExpr(node), _)) } - override predicate isBarrier(Expr e) { - super.isBarrier(e) - or - e.getUnspecifiedType() instanceof IntegralType - or + predicate isBarrier(DataFlow::Node node) { + node.asExpr().getUnspecifiedType() instanceof IntegralType + } + + predicate isBarrierIn(DataFlow::Node node) { exists(SqlBarrierFunction sql, int arg, FunctionInput input | - e = sql.getACallToThisFunction().getArgument(arg) and + node.asIndirectArgument() = sql.getACallToThisFunction().getArgument(arg) and input.isParameterDeref(arg) and sql.barrierSqlArgument(input, _) ) } } +module SqlTainted = TaintTracking::Global; + +import SqlTainted::PathGraph + from - SqlLikeFunction runSql, Expr taintedArg, Expr taintSource, PathNode sourceNode, PathNode sinkNode, - string taintCause, string callChain + SqlLikeFunction runSql, Expr taintedArg, FlowSource taintSource, SqlTainted::PathNode sourceNode, + SqlTainted::PathNode sinkNode, string callChain where runSql.outermostWrapperFunctionCall(taintedArg, callChain) and - taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and - isUserInput(taintSource, taintCause) + SqlTainted::flowPath(sourceNode, sinkNode) and + taintedArg = asSinkExpr(sinkNode.getNode()) and + taintSource = sourceNode.getNode() select taintedArg, sourceNode, sinkNode, "This argument to a SQL query function is derived from $@ and then passed to " + callChain + ".", - taintSource, "user input (" + taintCause + ")" + taintSource, "user input (" + taintSource.getSourceType() + ")" diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected index 4e7cfa96fa7dc..9497779d84bb6 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-089/SqlTainted/SqlTainted.expected @@ -1,25 +1,27 @@ 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 | -| 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 | -| 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 | -| test.cpp:43:27:43:30 | argv | test.cpp:43:27:43:33 | access to array | -subpaths +| test.c:14:27:14:30 | argv | test.c:21:18:21:23 | query1 indirection | +| test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection | +| test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection | +| test.cpp:39:27:39:30 | argv | 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 | +| test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection | +| test.cpp:39:27:39:30 | argv indirection | 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 | -| test.c:21:18:21:23 | query1 | semmle.label | query1 | -| test.c:21:18:21:23 | query1 | semmle.label | query1 | -| 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 | access to array | semmle.label | access to array | -| test.cpp:43:27:43:33 | access to array | semmle.label | access to array | +| test.c:14:27:14:30 | argv | semmle.label | argv | +| test.c:14:27:14:30 | argv indirection | semmle.label | argv indirection | +| test.c:14:27:14:30 | argv indirection | semmle.label | argv indirection | +| test.c:21:18:21:23 | query1 indirection | semmle.label | query1 indirection | +| test.cpp:39:27:39:30 | argv | semmle.label | argv | +| test.cpp:39:27:39:30 | argv indirection | semmle.label | argv indirection | +| test.cpp:39:27:39:30 | argv indirection | semmle.label | argv indirection | | 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 | +subpaths #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) | +| 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) | +| 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) | +| 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) | +| 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) | +| 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) | +| 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) | +| 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) |