Skip to content

Commit 257fe1a

Browse files
jketemajketema
authored
Merge pull request #14801 from jketema/rewrite-tainted-format-string
C++: Rewrite `cpp/tainted-format-string` away from `DefaultTaintTracking`
2 parents 98ddbe0 + 1fbe232 commit 257fe1a

File tree

7 files changed

+221
-467
lines changed

7 files changed

+221
-467
lines changed

‎cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,50 @@
1616
import cpp
1717
import semmle.code.cpp.security.Security
1818
import semmle.code.cpp.security.FunctionWithWrappers
19-
import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
20-
import TaintedWithPath
19+
import semmle.code.cpp.security.FlowSources
20+
import semmle.code.cpp.ir.dataflow.TaintTracking
21+
import semmle.code.cpp.ir.IR
22+
import Flow::PathGraph
2123

22-
class Configuration extends TaintTrackingConfiguration {
23-
override predicate isSink(Element tainted) {
24-
exists(PrintfLikeFunction printf | printf.outermostWrapperFunctionCall(tainted, _))
24+
predicate isSource(FlowSource source, string sourceType) {
25+
not source instanceof DataFlow::ExprNode and
26+
sourceType = source.getSourceType()
27+
}
28+
29+
module Config implements DataFlow::ConfigSig {
30+
predicate isSource(DataFlow::Node node) { isSource(node, _) }
31+
32+
predicate isSink(DataFlow::Node node) {
33+
exists(PrintfLikeFunction printf |
34+
printf.outermostWrapperFunctionCall([node.asExpr(), node.asIndirectExpr()], _)
35+
)
36+
}
37+
38+
private predicate isArithmeticNonCharType(ArithmeticType type) {
39+
not type instanceof CharType and
40+
not type instanceof Char8Type and
41+
not type instanceof Char16Type and
42+
not type instanceof Char32Type
43+
}
44+
45+
predicate isBarrier(DataFlow::Node node) {
46+
isSink(node) and isArithmeticNonCharType(node.asExpr().getUnspecifiedType())
47+
or
48+
isArithmeticNonCharType(node.asInstruction().(StoreInstruction).getResultType())
2549
}
2650
}
2751

52+
module Flow = TaintTracking::Global<Config>;
53+
2854
from
29-
PrintfLikeFunction printf, Expr arg, PathNode sourceNode, PathNode sinkNode,
30-
string printfFunction, Expr userValue, string cause
55+
PrintfLikeFunction printf, string printfFunction, string sourceType, DataFlow::Node source,
56+
DataFlow::Node sink, Flow::PathNode sourceNode, Flow::PathNode sinkNode
3157
where
32-
printf.outermostWrapperFunctionCall(arg, printfFunction) and
33-
taintedWithPath(userValue, arg, sourceNode, sinkNode) and
34-
isUserInput(userValue, cause)
35-
select arg, sourceNode, sinkNode,
58+
source = sourceNode.getNode() and
59+
sink = sinkNode.getNode() and
60+
isSource(source, sourceType) and
61+
printf.outermostWrapperFunctionCall([sink.asExpr(), sink.asIndirectExpr()], printfFunction) and
62+
Flow::flowPath(sourceNode, sinkNode)
63+
select sink, sourceNode, sinkNode,
3664
"The value of this argument may come from $@ and is being used as a formatting argument to " +
37-
printfFunction + ".", userValue, cause
65+
printfFunction + ".", source, sourceType

‎cpp/ql/test/query-tests/Security/CWE/CWE-134/SAMATE/UncontrolledFormatString.expected

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,16 @@
11
edges
2-
| char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data |
3-
| char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data |
4-
| char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data |
5-
| char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data |
6-
| char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | char_console_fprintf_01_bad.c:49:21:49:24 | data |
7-
| char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | char_console_fprintf_01_bad.c:49:21:49:24 | data |
8-
| char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | char_console_fprintf_01_bad.c:49:21:49:24 | data |
9-
| char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | char_console_fprintf_01_bad.c:49:21:49:24 | data |
10-
| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data |
11-
| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data |
12-
| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data |
13-
| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data |
14-
subpaths
2+
| char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data indirection |
3+
| char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | char_console_fprintf_01_bad.c:49:21:49:24 | data indirection |
4+
| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv indirection | char_environment_fprintf_01_bad.c:36:21:36:24 | data indirection |
155
nodes
166
| char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | semmle.label | recv output argument |
17-
| char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | semmle.label | ... + ... |
18-
| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data | semmle.label | data |
19-
| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data | semmle.label | data |
20-
| char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | semmle.label | ... + ... |
7+
| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data indirection | semmle.label | data indirection |
218
| char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | semmle.label | fgets output argument |
22-
| char_console_fprintf_01_bad.c:49:21:49:24 | data | semmle.label | data |
23-
| char_console_fprintf_01_bad.c:49:21:49:24 | data | semmle.label | data |
24-
| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | semmle.label | call to getenv |
25-
| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | semmle.label | call to getenv |
26-
| char_environment_fprintf_01_bad.c:36:21:36:24 | data | semmle.label | data |
27-
| char_environment_fprintf_01_bad.c:36:21:36:24 | data | semmle.label | data |
9+
| char_console_fprintf_01_bad.c:49:21:49:24 | data indirection | semmle.label | data indirection |
10+
| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv indirection | semmle.label | call to getenv indirection |
11+
| char_environment_fprintf_01_bad.c:36:21:36:24 | data indirection | semmle.label | data indirection |
12+
subpaths
2813
#select
29-
| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data | char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data | The value of this argument may come from $@ and is being used as a formatting argument to badVaSink(data), which calls vsnprintf(format). | char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | recv |
30-
| char_console_fprintf_01_bad.c:49:21:49:24 | data | char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | char_console_fprintf_01_bad.c:49:21:49:24 | data | The value of this argument may come from $@ and is being used as a formatting argument to fprintf(format). | char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | fgets |
31-
| char_environment_fprintf_01_bad.c:36:21:36:24 | data | char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data | The value of this argument may come from $@ and is being used as a formatting argument to fprintf(format). | char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | getenv |
14+
| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data indirection | char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data indirection | The value of this argument may come from $@ and is being used as a formatting argument to badVaSink(data), which calls vsnprintf(format). | char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | buffer read by recv |
15+
| char_console_fprintf_01_bad.c:49:21:49:24 | data indirection | char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | char_console_fprintf_01_bad.c:49:21:49:24 | data indirection | The value of this argument may come from $@ and is being used as a formatting argument to fprintf(format). | char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | string read by fgets |
16+
| char_environment_fprintf_01_bad.c:36:21:36:24 | data indirection | char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv indirection | char_environment_fprintf_01_bad.c:36:21:36:24 | data indirection | The value of this argument may come from $@ and is being used as a formatting argument to fprintf(format). | char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv indirection | an environment variable |

‎cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/argv/argvLocal.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ int main(int argc, char **argv) {
164164
printf(i91);
165165
printWrapper(i91);
166166

167-
// BAD: i10 value comes from argv
167+
// BAD: i10 value comes from argv [NOT DETECTED]
168168
int i10 = (int) argv[1];
169169
printf((char *) i10);
170170
printWrapper((char *) i10);

0 commit comments

Comments
 (0)