Skip to content

Commit dc76e3c

Browse files
committed
Convert RULE-21-14 to the new dataflow library
Observe that the special case for global variables is no longer needed, as these are properly handled in the new dataflow library.
1 parent 7b5eba0 commit dc76e3c

File tree

2 files changed

+61
-49
lines changed

2 files changed

+61
-49
lines changed

c/misra/src/rules/RULE-21-14/MemcmpUsedToCompareNullTerminatedStrings.ql

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@
1616
import cpp
1717
import codingstandards.c.misra
1818
import codingstandards.c.misra.EssentialTypes
19-
import semmle.code.cpp.dataflow.TaintTracking
19+
import semmle.code.cpp.dataflow.new.TaintTracking
2020
import NullTerminatedStringToMemcmpFlow::PathGraph
2121

2222
// Data flow from a StringLiteral or from an array of characters, to a memcmp call
2323
module NullTerminatedStringToMemcmpConfig implements DataFlow::ConfigSig {
2424
predicate isSource(DataFlow::Node source) {
25-
source.asExpr() instanceof StringLiteral
25+
source.asIndirectExpr(1) instanceof StringLiteral
2626
or
2727
exists(Variable v, ArrayAggregateLiteral aal |
2828
aal = v.getInitializer().getExpr() and
@@ -31,26 +31,14 @@ module NullTerminatedStringToMemcmpConfig implements DataFlow::ConfigSig {
3131
// Includes a null terminator somewhere in the array initializer
3232
aal.getAnElementExpr(_).getValue().toInt() = 0
3333
|
34-
// For local variables, use the array aggregate literal as the source
3534
aal = source.asExpr()
36-
or
37-
// ArrayAggregateLiterals used as initializers for global variables are not viable sources
38-
// for global data flow, so we instead report variable accesses as sources, where the variable
39-
// is constant or is not assigned in the program
40-
v instanceof GlobalVariable and
41-
source.asExpr() = v.getAnAccess() and
42-
(
43-
v.isConst()
44-
or
45-
not exists(Expr e | e = v.getAnAssignedValue() and not e = aal)
46-
)
4735
)
4836
}
4937

5038
predicate isSink(DataFlow::Node sink) {
5139
exists(FunctionCall memcmp |
5240
memcmp.getTarget().hasGlobalOrStdName("memcmp") and
53-
sink.asExpr() = memcmp.getArgument([0, 1])
41+
sink.asIndirectExpr() = memcmp.getArgument([0, 1])
5442
)
5543
}
5644
}
@@ -67,8 +55,8 @@ from
6755
where
6856
not isExcluded(memcmp, EssentialTypesPackage::memcmpUsedToCompareNullTerminatedStringsQuery()) and
6957
memcmp.getTarget().hasGlobalOrStdName("memcmp") and
70-
arg1.getNode().asExpr() = memcmp.getArgument(0) and
71-
arg2.getNode().asExpr() = memcmp.getArgument(1) and
58+
arg1.getNode().asIndirectExpr(1) = memcmp.getArgument(0) and
59+
arg2.getNode().asIndirectExpr(1) = memcmp.getArgument(1) and
7260
// There is a path from a null-terminated string to each argument
7361
NullTerminatedStringToMemcmpFlow::flowPath(source1, arg1) and
7462
NullTerminatedStringToMemcmpFlow::flowPath(source2, arg2) and
Lines changed: 56 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,62 @@
1-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (MemcmpUsedToCompareNullTerminatedStrings.ql:23,54-62)
2-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (MemcmpUsedToCompareNullTerminatedStrings.ql:24,22-30)
3-
WARNING: module 'DataFlow' has been deprecated and may be removed in future (MemcmpUsedToCompareNullTerminatedStrings.ql:50,20-28)
4-
WARNING: module 'TaintTracking' has been deprecated and may be removed in future (MemcmpUsedToCompareNullTerminatedStrings.ql:58,43-56)
51
edges
6-
| test.c:12:13:12:15 | a | test.c:14:10:14:10 | a | provenance | |
7-
| test.c:12:13:12:15 | a | test.c:23:13:23:13 | a | provenance | |
8-
| test.c:12:13:12:15 | a | test.c:24:10:24:10 | a | provenance | |
9-
| test.c:13:13:13:15 | b | test.c:14:13:14:13 | b | provenance | |
10-
| test.c:18:15:18:28 | {...} | test.c:21:10:21:10 | e | provenance | |
11-
| test.c:19:15:19:28 | {...} | test.c:21:13:21:13 | f | provenance | |
2+
| test.c:6:6:6:6 | *c | test.c:6:15:6:17 | 97 | provenance | |
3+
| test.c:6:6:6:6 | *c | test.c:16:10:16:10 | *c | provenance | |
4+
| test.c:6:6:6:6 | *c | test.c:26:13:26:13 | *c | provenance | |
5+
| test.c:6:6:6:6 | *c | test.c:27:10:27:10 | *c | provenance | |
6+
| test.c:6:14:6:26 | {...} | test.c:6:6:6:6 | *c | provenance | |
7+
| test.c:6:15:6:17 | 97 | test.c:6:20:6:22 | 98 | provenance | |
8+
| test.c:6:20:6:22 | 98 | test.c:6:25:6:25 | {...} | provenance | |
9+
| test.c:6:25:6:25 | {...} | test.c:6:14:6:26 | {...} | provenance | |
10+
| test.c:7:6:7:6 | *d | test.c:7:15:7:17 | 97 | provenance | |
11+
| test.c:7:6:7:6 | *d | test.c:16:13:16:13 | *d | provenance | |
12+
| test.c:7:14:7:26 | {...} | test.c:7:6:7:6 | *d | provenance | |
13+
| test.c:7:15:7:17 | 97 | test.c:7:20:7:22 | 98 | provenance | |
14+
| test.c:7:20:7:22 | 98 | test.c:7:25:7:25 | {...} | provenance | |
15+
| test.c:7:25:7:25 | {...} | test.c:7:14:7:26 | {...} | provenance | |
16+
| test.c:12:13:12:15 | *a | test.c:14:10:14:10 | *a | provenance | DataFlowFunction |
17+
| test.c:12:13:12:15 | *a | test.c:23:13:23:13 | *a | provenance | DataFlowFunction |
18+
| test.c:12:13:12:15 | *a | test.c:24:10:24:10 | *a | provenance | DataFlowFunction |
19+
| test.c:13:13:13:15 | *b | test.c:14:13:14:13 | *b | provenance | DataFlowFunction |
20+
| test.c:18:15:18:28 | {...} | test.c:21:10:21:10 | *e | provenance | |
21+
| test.c:18:27:18:27 | {...} | test.c:18:15:18:28 | {...} | provenance | |
22+
| test.c:19:15:19:28 | {...} | test.c:21:13:21:13 | *f | provenance | |
23+
| test.c:19:27:19:27 | {...} | test.c:19:15:19:28 | {...} | provenance | |
1224
nodes
13-
| test.c:10:10:10:12 | a | semmle.label | a |
14-
| test.c:10:15:10:17 | b | semmle.label | b |
15-
| test.c:12:13:12:15 | a | semmle.label | a |
16-
| test.c:13:13:13:15 | b | semmle.label | b |
17-
| test.c:14:10:14:10 | a | semmle.label | a |
18-
| test.c:14:13:14:13 | b | semmle.label | b |
19-
| test.c:16:10:16:10 | c | semmle.label | c |
20-
| test.c:16:13:16:13 | d | semmle.label | d |
25+
| test.c:6:6:6:6 | *c | semmle.label | *c |
26+
| test.c:6:14:6:26 | {...} | semmle.label | {...} |
27+
| test.c:6:15:6:17 | 97 | semmle.label | 97 |
28+
| test.c:6:20:6:22 | 98 | semmle.label | 98 |
29+
| test.c:6:25:6:25 | {...} | semmle.label | {...} |
30+
| test.c:7:6:7:6 | *d | semmle.label | *d |
31+
| test.c:7:14:7:26 | {...} | semmle.label | {...} |
32+
| test.c:7:15:7:17 | 97 | semmle.label | 97 |
33+
| test.c:7:20:7:22 | 98 | semmle.label | 98 |
34+
| test.c:7:25:7:25 | {...} | semmle.label | {...} |
35+
| test.c:10:10:10:12 | *a | semmle.label | *a |
36+
| test.c:10:15:10:17 | *b | semmle.label | *b |
37+
| test.c:12:13:12:15 | *a | semmle.label | *a |
38+
| test.c:13:13:13:15 | *b | semmle.label | *b |
39+
| test.c:14:10:14:10 | *a | semmle.label | *a |
40+
| test.c:14:13:14:13 | *b | semmle.label | *b |
41+
| test.c:16:10:16:10 | *c | semmle.label | *c |
42+
| test.c:16:13:16:13 | *d | semmle.label | *d |
2143
| test.c:18:15:18:28 | {...} | semmle.label | {...} |
44+
| test.c:18:27:18:27 | {...} | semmle.label | {...} |
2245
| test.c:19:15:19:28 | {...} | semmle.label | {...} |
23-
| test.c:21:10:21:10 | e | semmle.label | e |
24-
| test.c:21:13:21:13 | f | semmle.label | f |
25-
| test.c:23:13:23:13 | a | semmle.label | a |
26-
| test.c:24:10:24:10 | a | semmle.label | a |
27-
| test.c:26:13:26:13 | c | semmle.label | c |
28-
| test.c:27:10:27:10 | c | semmle.label | c |
46+
| test.c:19:27:19:27 | {...} | semmle.label | {...} |
47+
| test.c:21:10:21:10 | *e | semmle.label | *e |
48+
| test.c:21:13:21:13 | *f | semmle.label | *f |
49+
| test.c:23:13:23:13 | *a | semmle.label | *a |
50+
| test.c:24:10:24:10 | *a | semmle.label | *a |
51+
| test.c:26:13:26:13 | *c | semmle.label | *c |
52+
| test.c:27:10:27:10 | *c | semmle.label | *c |
2953
subpaths
3054
#select
31-
| test.c:10:3:10:8 | call to memcmp | test.c:10:10:10:12 | a | test.c:10:10:10:12 | a | memcmp used to compare $@ with $@. | test.c:10:10:10:12 | a | null-terminated string | test.c:10:15:10:17 | b | null-terminated string |
32-
| test.c:10:3:10:8 | call to memcmp | test.c:10:15:10:17 | b | test.c:10:15:10:17 | b | memcmp used to compare $@ with $@. | test.c:10:10:10:12 | a | null-terminated string | test.c:10:15:10:17 | b | null-terminated string |
33-
| test.c:14:3:14:8 | call to memcmp | test.c:12:13:12:15 | a | test.c:14:10:14:10 | a | memcmp used to compare $@ with $@. | test.c:12:13:12:15 | a | null-terminated string | test.c:13:13:13:15 | b | null-terminated string |
34-
| test.c:14:3:14:8 | call to memcmp | test.c:13:13:13:15 | b | test.c:14:13:14:13 | b | memcmp used to compare $@ with $@. | test.c:12:13:12:15 | a | null-terminated string | test.c:13:13:13:15 | b | null-terminated string |
35-
| test.c:16:3:16:8 | call to memcmp | test.c:16:10:16:10 | c | test.c:16:10:16:10 | c | memcmp used to compare $@ with $@. | test.c:16:10:16:10 | c | null-terminated string | test.c:16:13:16:13 | d | null-terminated string |
36-
| test.c:16:3:16:8 | call to memcmp | test.c:16:13:16:13 | d | test.c:16:13:16:13 | d | memcmp used to compare $@ with $@. | test.c:16:10:16:10 | c | null-terminated string | test.c:16:13:16:13 | d | null-terminated string |
37-
| test.c:21:3:21:8 | call to memcmp | test.c:18:15:18:28 | {...} | test.c:21:10:21:10 | e | memcmp used to compare $@ with $@. | test.c:18:15:18:28 | {...} | null-terminated string | test.c:19:15:19:28 | {...} | null-terminated string |
38-
| test.c:21:3:21:8 | call to memcmp | test.c:19:15:19:28 | {...} | test.c:21:13:21:13 | f | memcmp used to compare $@ with $@. | test.c:18:15:18:28 | {...} | null-terminated string | test.c:19:15:19:28 | {...} | null-terminated string |
55+
| test.c:10:3:10:8 | call to memcmp | test.c:10:10:10:12 | *a | test.c:10:10:10:12 | *a | memcmp used to compare $@ with $@. | test.c:10:10:10:12 | *a | null-terminated string | test.c:10:15:10:17 | *b | null-terminated string |
56+
| test.c:10:3:10:8 | call to memcmp | test.c:10:15:10:17 | *b | test.c:10:15:10:17 | *b | memcmp used to compare $@ with $@. | test.c:10:10:10:12 | *a | null-terminated string | test.c:10:15:10:17 | *b | null-terminated string |
57+
| test.c:14:3:14:8 | call to memcmp | test.c:12:13:12:15 | *a | test.c:14:10:14:10 | *a | memcmp used to compare $@ with $@. | test.c:12:13:12:15 | *a | null-terminated string | test.c:13:13:13:15 | *b | null-terminated string |
58+
| test.c:14:3:14:8 | call to memcmp | test.c:13:13:13:15 | *b | test.c:14:13:14:13 | *b | memcmp used to compare $@ with $@. | test.c:12:13:12:15 | *a | null-terminated string | test.c:13:13:13:15 | *b | null-terminated string |
59+
| test.c:16:3:16:8 | call to memcmp | test.c:6:25:6:25 | {...} | test.c:16:10:16:10 | *c | memcmp used to compare $@ with $@. | test.c:6:25:6:25 | {...} | null-terminated string | test.c:7:25:7:25 | {...} | null-terminated string |
60+
| test.c:16:3:16:8 | call to memcmp | test.c:7:25:7:25 | {...} | test.c:16:13:16:13 | *d | memcmp used to compare $@ with $@. | test.c:6:25:6:25 | {...} | null-terminated string | test.c:7:25:7:25 | {...} | null-terminated string |
61+
| test.c:21:3:21:8 | call to memcmp | test.c:18:27:18:27 | {...} | test.c:21:10:21:10 | *e | memcmp used to compare $@ with $@. | test.c:18:27:18:27 | {...} | null-terminated string | test.c:19:27:19:27 | {...} | null-terminated string |
62+
| test.c:21:3:21:8 | call to memcmp | test.c:19:27:19:27 | {...} | test.c:21:13:21:13 | *f | memcmp used to compare $@ with $@. | test.c:18:27:18:27 | {...} | null-terminated string | test.c:19:27:19:27 | {...} | null-terminated string |

0 commit comments

Comments
 (0)