Skip to content

Commit 0159cd4

Browse files
jketemajketema
committed
C++: Rewrite cpp/path-injection to not use DefaultTaintTracking 
1 parent 53b86fd commit 0159cd4

3 files changed

Lines changed: 39 additions & 29 deletions

File tree

‎cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* @description Accessing paths influenced by users can allow an
44
* attacker to access unexpected resources.
55
* @kind path-problem
6+
* @precision medium
67
* @problem.severity warning
78
* @security-severity 7.5
89
* @precision medium
@@ -17,8 +18,9 @@
1718
import cpp
1819
import semmle.code.cpp.security.FunctionWithWrappers
1920
import semmle.code.cpp.security.Security
20-
import semmle.code.cpp.security.TaintTracking
21-
import TaintedWithPath
21+
import semmle.code.cpp.ir.IR
22+
import semmle.code.cpp.ir.dataflow.TaintTracking
23+
import DataFlow::PathGraph
2224

2325
/**
2426
* A function for opening a file.
@@ -46,18 +48,42 @@ class FileFunction extends FunctionWithWrappers {
4648
override predicate interestingArg(int arg) { arg = 0 }
4749
}
4850

49-
class TaintedPathConfiguration extends TaintTrackingConfiguration {
50-
override predicate isSink(Element tainted) {
51-
exists(FileFunction fileFunction | fileFunction.outermostWrapperFunctionCall(tainted, _))
51+
Expr asSourceExpr(DataFlow::Node node) {
52+
result in [node.asConvertedExpr(), node.asDefiningArgument()]
53+
}
54+
55+
Expr asSinkExpr(DataFlow::Node node) {
56+
result = node.asConvertedExpr()
57+
or
58+
result =
59+
node.asOperand()
60+
.(SideEffectOperand)
61+
.getUse()
62+
.(ReadSideEffectInstruction)
63+
.getArgumentDef()
64+
.getUnconvertedResultExpression()
65+
}
66+
67+
class TaintedPathConfiguration extends TaintTracking::Configuration {
68+
TaintedPathConfiguration() { this = "TaintedPathConfiguration" }
69+
70+
override predicate isSource(DataFlow::Node node) { isUserInput(asSourceExpr(node), _) }
71+
72+
override predicate isSink(DataFlow::Node node) {
73+
exists(FileFunction fileFunction |
74+
fileFunction.outermostWrapperFunctionCall(asSinkExpr(node), _)
75+
)
5276
}
5377
}
5478

5579
from
56-
FileFunction fileFunction, Expr taintedArg, Expr taintSource, PathNode sourceNode,
57-
PathNode sinkNode, string taintCause, string callChain
80+
FileFunction fileFunction, Expr taintedArg, Expr taintSource, TaintedPathConfiguration cfg,
81+
DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, string taintCause, string callChain
5882
where
83+
taintedArg = asSinkExpr(sinkNode.getNode()) and
5984
fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and
60-
taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
85+
cfg.hasFlowPath(sourceNode, sinkNode) and
86+
taintSource = asSourceExpr(sourceNode.getNode()) and
6187
isUserInput(taintSource, taintCause)
6288
select taintedArg, sourceNode, sinkNode,
6389
"This argument to a file access function is derived from $@ and then passed to " + callChain + ".",

‎cpp/ql/test/query-tests/Security/CWE/CWE-022/SAMATE/TaintedPath/TaintedPath.expected‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,11 @@
11
edges
2-
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | ... + ... | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | (const char *)... |
3-
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | ... + ... | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data |
4-
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | ... + ... | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data |
5-
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | ... + ... | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data indirection |
6-
| 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 | (const char *)... |
7-
| 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 | data |
82
| 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 | data |
93
| 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 | data indirection |
10-
subpaths
114
nodes
12-
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | ... + ... | semmle.label | ... + ... |
135
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:55:27:55:38 | fgets output argument | semmle.label | fgets output argument |
14-
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | (const char *)... | semmle.label | (const char *)... |
15-
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data | semmle.label | data |
166
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data | semmle.label | data |
177
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data indirection | semmle.label | data indirection |
8+
subpaths
189
#select
19-
| 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 | ... + ... | CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data | 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 | ... + ... | user input (fgets) |
10+
| 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 | data | 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 | ... + ... | user input (fgets) |
11+
| 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 | data 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 | ... + ... | user input (fgets) |

‎cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,11 @@
11
edges
2-
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | (const char *)... |
3-
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | (const char *)... |
42
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName |
5-
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName |
6-
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName |
7-
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName |
8-
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName indirection |
93
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName indirection |
10-
subpaths
114
nodes
125
| test.c:9:23:9:26 | argv | semmle.label | argv |
13-
| test.c:9:23:9:26 | argv | semmle.label | argv |
14-
| test.c:17:11:17:18 | (const char *)... | semmle.label | (const char *)... |
15-
| test.c:17:11:17:18 | fileName | semmle.label | fileName |
166
| test.c:17:11:17:18 | fileName | semmle.label | fileName |
177
| test.c:17:11:17:18 | fileName indirection | semmle.label | fileName indirection |
8+
subpaths
189
#select
1910
| test.c:17:11:17:18 | fileName | test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName | 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 (argv) |
11+
| test.c:17:11:17:18 | fileName | test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName 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 (argv) |

0 commit comments

Comments
 (0)