From 4075f693bd94d8e7e230ac705403d3a4ebe65615 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Wed, 14 Dec 2022 11:27:13 +0100 Subject: [PATCH] C++: Make `cpp/path-injection` work with use-use dataflow --- .../src/Security/CWE/CWE-022/TaintedPath.ql | 14 ++------ .../SAMATE/TaintedPath/TaintedPath.expected | 4 +++ .../CWE-022/semmle/tests/TaintedPath.expected | 32 +++++++++++++++++++ .../Security/CWE/CWE-022/semmle/tests/test.c | 2 +- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql index f000e25df52..147cf73066a 100644 --- a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql @@ -47,16 +47,6 @@ class FileFunction extends FunctionWithWrappers { override predicate interestingArg(int arg) { arg = 0 } } -Expr asSinkExpr(DataFlow::Node node) { - result = - node.asOperand() - .(SideEffectOperand) - .getUse() - .(ReadSideEffectInstruction) - .getArgumentDef() - .getUnconvertedResultExpression() -} - /** * Holds for a variable that has any kind of upper-bound check anywhere in the program. * This is biased towards being inclusive and being a coarse overapproximation because @@ -87,7 +77,7 @@ class TaintedPathConfiguration extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node node) { exists(FileFunction fileFunction | - fileFunction.outermostWrapperFunctionCall(asSinkExpr(node), _) + fileFunction.outermostWrapperFunctionCall(node.asIndirectArgument(), _) ) } @@ -108,7 +98,7 @@ from FileFunction fileFunction, Expr taintedArg, FlowSource taintSource, TaintedPathConfiguration cfg, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, string callChain where - taintedArg = asSinkExpr(sinkNode.getNode()) and + taintedArg = sinkNode.getNode().asIndirectArgument() and fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and cfg.hasFlowPath(sourceNode, sinkNode) and taintSource = sourceNode.getNode() diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/SAMATE/TaintedPath/TaintedPath.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-022/SAMATE/TaintedPath/TaintedPath.expected index e217064d1df..c9c7fc404a3 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/SAMATE/TaintedPath/TaintedPath.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/SAMATE/TaintedPath/TaintedPath.expected @@ -1,4 +1,8 @@ edges +| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | fgets output argument | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | Convert indirection | nodes +| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | fgets output argument | semmle.label | fgets output argument | +| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | Convert indirection | semmle.label | Convert indirection | subpaths #select +| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | fgets output argument | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | Convert indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | fgets output argument | user input (string read by fgets) | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected index e217064d1df..4d3ac48d18a 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected @@ -1,4 +1,36 @@ edges +| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | Convert indirection | +| test.c:9:23:9:26 | argv indirection | test.c:17:11:17:18 | Convert indirection | +| test.c:9:23:9:26 | argv indirection | test.c:17:11:17:18 | Convert indirection | +| test.c:31:22:31:25 | argv indirection | test.c:32:11:32:18 | Convert indirection | +| test.c:31:22:31:25 | argv indirection | test.c:32:11:32:18 | Convert indirection | +| test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | Convert indirection | +| test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | Convert indirection | +| test.c:57:10:57:13 | argv indirection | test.c:57:10:57:16 | access to array indirection | +| test.c:57:10:57:13 | argv indirection | test.c:57:10:57:16 | access to array indirection | nodes +| test.c:9:23:9:26 | argv | semmle.label | argv | +| test.c:9:23:9:26 | argv indirection | semmle.label | argv indirection | +| test.c:9:23:9:26 | argv indirection | semmle.label | argv indirection | +| test.c:17:11:17:18 | Convert indirection | semmle.label | Convert indirection | +| test.c:31:22:31:25 | argv indirection | semmle.label | argv indirection | +| test.c:31:22:31:25 | argv indirection | semmle.label | argv indirection | +| test.c:32:11:32:18 | Convert indirection | semmle.label | Convert indirection | +| test.c:37:17:37:24 | scanf output argument | semmle.label | scanf output argument | +| test.c:38:11:38:18 | Convert indirection | semmle.label | Convert indirection | +| test.c:43:17:43:24 | scanf output argument | semmle.label | scanf output argument | +| test.c:44:11:44:18 | Convert indirection | semmle.label | Convert indirection | +| test.c:57:10:57:13 | argv indirection | semmle.label | argv indirection | +| test.c:57:10:57:13 | argv indirection | semmle.label | argv indirection | +| test.c:57:10:57:16 | access to array indirection | semmle.label | access to array indirection | subpaths #select +| test.c:17:11:17:18 | fileName | test.c:9:23:9:26 | argv | test.c:17:11:17:18 | Convert indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:9:23:9:26 | argv | user input (a command-line argument) | +| test.c:17:11:17:18 | fileName | test.c:9:23:9:26 | argv indirection | test.c:17:11:17:18 | Convert indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:9:23:9:26 | argv indirection | user input (a command-line argument) | +| test.c:17:11:17:18 | fileName | test.c:9:23:9:26 | argv indirection | test.c:17:11:17:18 | Convert indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:9:23:9:26 | argv indirection | user input (a command-line argument) | +| test.c:32:11:32:18 | fileName | test.c:31:22:31:25 | argv indirection | test.c:32:11:32:18 | Convert indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:31:22:31:25 | argv indirection | user input (a command-line argument) | +| test.c:32:11:32:18 | fileName | test.c:31:22:31:25 | argv indirection | test.c:32:11:32:18 | Convert indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:31:22:31:25 | argv indirection | user input (a command-line argument) | +| test.c:38:11:38:18 | fileName | test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | Convert indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:37:17:37:24 | scanf output argument | user input (value read by scanf) | +| test.c:44:11:44:18 | fileName | test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | Convert indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:43:17:43:24 | scanf output argument | user input (value read by scanf) | +| test.c:57:10:57:16 | access to array | test.c:57:10:57:13 | argv indirection | test.c:57:10:57:16 | access to array indirection | This argument to a file access function is derived from $@ and then passed to read(fileName), which calls fopen(filename). | test.c:57:10:57:13 | argv indirection | user input (a command-line argument) | +| test.c:57:10:57:16 | access to array | test.c:57:10:57:13 | argv indirection | test.c:57:10:57:16 | access to array indirection | This argument to a file access function is derived from $@ and then passed to read(fileName), which calls fopen(filename). | test.c:57:10:57:13 | argv indirection | user input (a command-line argument) | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c index a11fd73edd7..824db8f16ad 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/test.c @@ -54,7 +54,7 @@ int main(int argc, char** argv) { { void read(const char *fileName); - read(argv[1]); // BAD [NOT DETECTED] + read(argv[1]); // BAD } }