Skip to content

Commit a9577a3

Browse files
jketemajketema
committed
C++: Use FlowSource in cpp/path-injection 
1 parent 7d1f10b commit a9577a3

File tree

3 files changed

+12
-35
lines changed

3 files changed

+12
-35
lines changed

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

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
import cpp
1818
import semmle.code.cpp.security.FunctionWithWrappers
19-
import semmle.code.cpp.security.Security
19+
import semmle.code.cpp.security.FlowSources
2020
import semmle.code.cpp.ir.IR
2121
import semmle.code.cpp.ir.dataflow.TaintTracking
2222
import DataFlow::PathGraph
@@ -47,12 +47,6 @@ class FileFunction extends FunctionWithWrappers {
4747
override predicate interestingArg(int arg) { arg = 0 }
4848
}
4949

50-
Expr asSourceExpr(DataFlow::Node node) {
51-
result = node.asConvertedExpr()
52-
or
53-
result = node.asDefiningArgument()
54-
}
55-
5650
Expr asSinkExpr(DataFlow::Node node) {
5751
result =
5852
node.asOperand()
@@ -89,7 +83,7 @@ predicate hasUpperBoundsCheck(Variable var) {
8983
class TaintedPathConfiguration extends TaintTracking::Configuration {
9084
TaintedPathConfiguration() { this = "TaintedPathConfiguration" }
9185

92-
override predicate isSource(DataFlow::Node node) { isUserInput(asSourceExpr(node), _) }
86+
override predicate isSource(DataFlow::Node node) { node instanceof FlowSource }
9387

9488
override predicate isSink(DataFlow::Node node) {
9589
exists(FileFunction fileFunction |
@@ -108,31 +102,16 @@ class TaintedPathConfiguration extends TaintTracking::Configuration {
108102
hasUpperBoundsCheck(checkedVar)
109103
)
110104
}
111-
112-
predicate hasFilteredFlowPath(DataFlow::PathNode source, DataFlow::PathNode sink) {
113-
this.hasFlowPath(source, sink) and
114-
// The use of `isUserInput` in `isSink` in combination with `asSourceExpr` causes
115-
// duplicate results. Filter these duplicates. The proper solution is to switch to
116-
// using `LocalFlowSource` and `RemoteFlowSource`, but this currently only supports
117-
// a subset of the cases supported by `isUserInput`.
118-
not exists(DataFlow::PathNode source2 |
119-
this.hasFlowPath(source2, sink) and
120-
asSourceExpr(source.getNode()) = asSourceExpr(source2.getNode())
121-
|
122-
not exists(source.getNode().asConvertedExpr()) and exists(source2.getNode().asConvertedExpr())
123-
)
124-
}
125105
}
126106

127107
from
128-
FileFunction fileFunction, Expr taintedArg, Expr taintSource, TaintedPathConfiguration cfg,
129-
DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, string taintCause, string callChain
108+
FileFunction fileFunction, Expr taintedArg, FlowSource taintSource, TaintedPathConfiguration cfg,
109+
DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode, string callChain
130110
where
131111
taintedArg = asSinkExpr(sinkNode.getNode()) and
132112
fileFunction.outermostWrapperFunctionCall(taintedArg, callChain) and
133-
cfg.hasFilteredFlowPath(sourceNode, sinkNode) and
134-
taintSource = asSourceExpr(sourceNode.getNode()) and
135-
isUserInput(taintSource, taintCause)
113+
cfg.hasFlowPath(sourceNode, sinkNode) and
114+
taintSource = sourceNode.getNode()
136115
select taintedArg, sourceNode, sinkNode,
137116
"This argument to a file access function is derived from $@ and then passed to " + callChain + ".",
138-
taintSource, "user input (" + taintCause + ")"
117+
taintSource, "user input (" + taintSource.getSourceType() + ")"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ nodes
55
| CWE23_Relative_Path_Traversal__char_console_fopen_11.cpp:77:23:77:26 | data indirection | semmle.label | data indirection |
66
subpaths
77
#select
8-
| 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) |
8+
| 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 | fgets output argument | user input (String read by fgets) |

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ edges
22
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName indirection |
33
| test.c:31:22:31:25 | argv | test.c:32:11:32:18 | fileName indirection |
44
| test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | fileName indirection |
5-
| test.c:43:17:43:24 | fileName | test.c:44:11:44:18 | fileName indirection |
65
| test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | fileName indirection |
76
nodes
87
| test.c:9:23:9:26 | argv | semmle.label | argv |
@@ -11,12 +10,11 @@ nodes
1110
| test.c:32:11:32:18 | fileName indirection | semmle.label | fileName indirection |
1211
| test.c:37:17:37:24 | scanf output argument | semmle.label | scanf output argument |
1312
| test.c:38:11:38:18 | fileName indirection | semmle.label | fileName indirection |
14-
| test.c:43:17:43:24 | fileName | semmle.label | fileName |
1513
| test.c:43:17:43:24 | scanf output argument | semmle.label | scanf output argument |
1614
| test.c:44:11:44:18 | fileName indirection | semmle.label | fileName indirection |
1715
subpaths
1816
#select
19-
| 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) |
20-
| test.c:32:11:32:18 | fileName | test.c:31:22:31:25 | argv | test.c:32:11:32:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:31:22:31:25 | argv | user input (argv) |
21-
| test.c:38:11:38:18 | fileName | test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:37:17:37:24 | fileName | user input (scanf) |
22-
| test.c:44:11:44:18 | fileName | test.c:43:17:43:24 | fileName | test.c:44:11:44:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:43:17:43:24 | fileName | user input (scanf) |
17+
| 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 (a command-line argument) |
18+
| test.c:32:11:32:18 | fileName | test.c:31:22:31:25 | argv | test.c:32:11:32:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:31:22:31:25 | argv | user input (a command-line argument) |
19+
| test.c:38:11:38:18 | fileName | test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | fileName 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) |
20+
| test.c:44:11:44:18 | fileName | test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | fileName 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) |

0 commit comments

Comments
 (0)