Skip to content

Commit f97b6e2

Browse files
MathiasVPMathiasVP
committed
C++: Stop conflating pointers and indirections in the query.
1 parent 32e532f commit f97b6e2

File tree

4 files changed

+17
-38
lines changed

4 files changed

+17
-38
lines changed

‎cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ predicate isNonConst(DataFlow::Node node) {
7474
// Parameters of uncalled functions that aren't const
7575
exists(UncalledFunction f, Parameter p |
7676
f.getAParameter() = p and
77-
p = node.asParameter() and
77+
// We pick the indirection of the parameter since this query is focused
78+
// on strings.
79+
p = node.asParameter(1) and
7880
// Ignore main's argv parameter as it is already considered a `FlowSource`
7981
// not ignoring it will result in path redundancies
8082
(f.getName() = "main" implies p != f.getParameter(1))
@@ -116,7 +118,7 @@ predicate isNonConst(DataFlow::Node node) {
116118
c.getTarget().hasDefinition() and
117119
if node instanceof DataFlow::DefinitionByReferenceNode
118120
then c.getAnArgument() = node.asDefiningArgument()
119-
else c = [node.asExpr(), node.asIndirectExpr()]
121+
else c = node.asIndirectExpr()
120122
)
121123
}
122124

@@ -125,7 +127,7 @@ predicate isNonConst(DataFlow::Node node) {
125127
* `FormattingFunctionCall`.
126128
*/
127129
predicate isSinkImpl(DataFlow::Node sink, Expr formatString) {
128-
[sink.asExpr(), sink.asIndirectExpr()] = formatString and
130+
sink.asIndirectExpr() = formatString and
129131
exists(FormattingFunctionCall fc | formatString = fc.getArgument(fc.getFormatParameterIndex()))
130132
}
131133

@@ -136,7 +138,7 @@ module NonConstFlowConfig implements DataFlow::ConfigSig {
136138

137139
predicate isBarrier(DataFlow::Node node) {
138140
// Ignore tracing non-const through array indices
139-
exists(ArrayExpr a | a.getArrayOffset() = node.asExpr())
141+
exists(ArrayExpr a | a.getArrayOffset() = node.asIndirectExpr())
140142
}
141143
}
142144

‎cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/NonConstantFormat.expected

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,10 @@ edges
88
| nested.cpp:35:19:35:21 | *fmt | nested.cpp:27:32:27:34 | *fmt | provenance | |
99
| nested.cpp:42:24:42:34 | *call to ext_fmt_str | nested.cpp:34:37:34:39 | *fmt | provenance | |
1010
| nested.cpp:86:19:86:46 | *call to __builtin_alloca | nested.cpp:87:18:87:20 | *fmt | provenance | |
11-
| test.cpp:27:39:27:39 | n | test.cpp:27:13:27:24 | **make_message | provenance | |
12-
| test.cpp:46:14:46:17 | argc | test.cpp:51:23:51:30 | ... - ... | provenance | |
1311
| test.cpp:46:27:46:30 | **argv | test.cpp:130:20:130:26 | *access to array | provenance | |
14-
| test.cpp:51:23:51:30 | ... - ... | test.cpp:27:39:27:39 | n | provenance | |
15-
| test.cpp:51:23:51:30 | ... - ... | test.cpp:51:10:51:21 | *call to make_message | provenance | |
16-
| test.cpp:155:27:155:30 | data | test.cpp:157:12:157:15 | data | provenance | |
17-
| test.cpp:167:31:167:34 | data | test.cpp:170:12:170:14 | *res | provenance | |
18-
| test.cpp:193:32:193:34 | str | test.cpp:195:31:195:33 | str | provenance | |
19-
| test.cpp:193:32:193:34 | str | test.cpp:197:11:197:14 | *wstr | provenance | |
12+
| test.cpp:167:31:167:34 | *data | test.cpp:170:12:170:14 | *res | provenance | |
13+
| test.cpp:193:32:193:34 | *str | test.cpp:195:31:195:33 | *str | provenance | |
14+
| test.cpp:193:32:193:34 | *str | test.cpp:197:11:197:14 | *wstr | provenance | |
2015
| test.cpp:204:25:204:36 | *call to get_string | test.cpp:205:12:205:20 | *... + ... | provenance | |
2116
| test.cpp:204:25:204:36 | *call to get_string | test.cpp:206:12:206:16 | *hello | provenance | |
2217
| test.cpp:209:25:209:36 | *call to get_string | test.cpp:211:12:211:16 | *hello | provenance | |
@@ -42,19 +37,12 @@ nodes
4237
| nested.cpp:79:32:79:38 | *call to get_fmt | semmle.label | *call to get_fmt |
4338
| nested.cpp:86:19:86:46 | *call to __builtin_alloca | semmle.label | *call to __builtin_alloca |
4439
| nested.cpp:87:18:87:20 | *fmt | semmle.label | *fmt |
45-
| test.cpp:27:13:27:24 | **make_message | semmle.label | **make_message |
46-
| test.cpp:27:39:27:39 | n | semmle.label | n |
47-
| test.cpp:46:14:46:17 | argc | semmle.label | argc |
4840
| test.cpp:46:27:46:30 | **argv | semmle.label | **argv |
49-
| test.cpp:51:10:51:21 | *call to make_message | semmle.label | *call to make_message |
50-
| test.cpp:51:23:51:30 | ... - ... | semmle.label | ... - ... |
5141
| test.cpp:130:20:130:26 | *access to array | semmle.label | *access to array |
52-
| test.cpp:155:27:155:30 | data | semmle.label | data |
53-
| test.cpp:157:12:157:15 | data | semmle.label | data |
54-
| test.cpp:167:31:167:34 | data | semmle.label | data |
42+
| test.cpp:167:31:167:34 | *data | semmle.label | *data |
5543
| test.cpp:170:12:170:14 | *res | semmle.label | *res |
56-
| test.cpp:193:32:193:34 | str | semmle.label | str |
57-
| test.cpp:195:31:195:33 | str | semmle.label | str |
44+
| test.cpp:193:32:193:34 | *str | semmle.label | *str |
45+
| test.cpp:195:31:195:33 | *str | semmle.label | *str |
5846
| test.cpp:197:11:197:14 | *wstr | semmle.label | *wstr |
5947
| test.cpp:204:25:204:36 | *call to get_string | semmle.label | *call to get_string |
6048
| test.cpp:205:12:205:20 | *... + ... | semmle.label | *... + ... |
@@ -74,20 +62,17 @@ nodes
7462
| test.cpp:245:25:245:36 | *call to get_string | semmle.label | *call to get_string |
7563
| test.cpp:247:12:247:16 | *hello | semmle.label | *hello |
7664
subpaths
77-
| test.cpp:51:23:51:30 | ... - ... | test.cpp:27:39:27:39 | n | test.cpp:27:13:27:24 | **make_message | test.cpp:51:10:51:21 | *call to make_message |
7865
#select
7966
| NonConstantFormat.c:30:10:30:16 | *access to array | NonConstantFormat.c:28:27:28:30 | **argv | NonConstantFormat.c:30:10:30:16 | *access to array | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | NonConstantFormat.c:30:3:30:8 | call to printf | printf |
8067
| NonConstantFormat.c:41:9:41:45 | *call to any_random_function | NonConstantFormat.c:41:9:41:45 | *call to any_random_function | NonConstantFormat.c:41:9:41:45 | *call to any_random_function | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | NonConstantFormat.c:41:2:41:7 | call to printf | printf |
8168
| NonConstantFormat.c:45:9:45:48 | *call to gettext | NonConstantFormat.c:45:11:45:47 | *call to any_random_function | NonConstantFormat.c:45:9:45:48 | *call to gettext | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | NonConstantFormat.c:45:2:45:7 | call to printf | printf |
8269
| nested.cpp:21:23:21:26 | *fmt0 | nested.cpp:42:24:42:34 | *call to ext_fmt_str | nested.cpp:21:23:21:26 | *fmt0 | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | nested.cpp:21:5:21:12 | call to snprintf | snprintf |
8370
| nested.cpp:79:32:79:38 | *call to get_fmt | nested.cpp:79:32:79:38 | *call to get_fmt | nested.cpp:79:32:79:38 | *call to get_fmt | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | nested.cpp:79:5:79:14 | call to diagnostic | diagnostic |
8471
| nested.cpp:87:18:87:20 | *fmt | nested.cpp:86:19:86:46 | *call to __builtin_alloca | nested.cpp:87:18:87:20 | *fmt | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | nested.cpp:87:7:87:16 | call to diagnostic | diagnostic |
85-
| test.cpp:51:10:51:21 | *call to make_message | test.cpp:46:14:46:17 | argc | test.cpp:51:10:51:21 | *call to make_message | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:51:3:51:8 | call to printf | printf |
8672
| test.cpp:130:20:130:26 | *access to array | test.cpp:46:27:46:30 | **argv | test.cpp:130:20:130:26 | *access to array | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:130:2:130:10 | call to sprintf | sprintf |
87-
| test.cpp:157:12:157:15 | data | test.cpp:155:27:155:30 | data | test.cpp:157:12:157:15 | data | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:157:5:157:10 | call to printf | printf |
88-
| test.cpp:170:12:170:14 | *res | test.cpp:167:31:167:34 | data | test.cpp:170:12:170:14 | *res | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:170:5:170:10 | call to printf | printf |
89-
| test.cpp:195:31:195:33 | str | test.cpp:193:32:193:34 | str | test.cpp:195:31:195:33 | str | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:195:3:195:18 | call to StringCchPrintfW | StringCchPrintfW |
90-
| test.cpp:197:11:197:14 | *wstr | test.cpp:193:32:193:34 | str | test.cpp:197:11:197:14 | *wstr | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:197:3:197:9 | call to wprintf | wprintf |
73+
| test.cpp:170:12:170:14 | *res | test.cpp:167:31:167:34 | *data | test.cpp:170:12:170:14 | *res | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:170:5:170:10 | call to printf | printf |
74+
| test.cpp:195:31:195:33 | *str | test.cpp:193:32:193:34 | *str | test.cpp:195:31:195:33 | *str | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:195:3:195:18 | call to StringCchPrintfW | StringCchPrintfW |
75+
| test.cpp:197:11:197:14 | *wstr | test.cpp:193:32:193:34 | *str | test.cpp:197:11:197:14 | *wstr | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:197:3:197:9 | call to wprintf | wprintf |
9176
| test.cpp:205:12:205:20 | *... + ... | test.cpp:204:25:204:36 | *call to get_string | test.cpp:205:12:205:20 | *... + ... | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:205:5:205:10 | call to printf | printf |
9277
| test.cpp:206:12:206:16 | *hello | test.cpp:204:25:204:36 | *call to get_string | test.cpp:206:12:206:16 | *hello | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:206:5:206:10 | call to printf | printf |
9378
| test.cpp:211:12:211:16 | *hello | test.cpp:209:25:209:36 | *call to get_string | test.cpp:211:12:211:16 | *hello | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:211:5:211:10 | call to printf | printf |

‎cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ int main(int argc, char **argv) {
4848
printf(choose_message(argc - 1), argc - 1); // GOOD
4949
printf(messages[1]); // GOOD
5050
printf(message); // GOOD
51-
printf(make_message(argc - 1)); // BAD
51+
printf(make_message(argc - 1)); // BAD [NOT DETECTED]
5252
printf("Hello, World\n"); // GOOD
5353
printf(_("Hello, World\n")); // GOOD
5454
{
@@ -154,7 +154,7 @@ void print_ith_message() {
154154

155155
void fmt_via_strcpy(char *data) {
156156
strcpy(data, "some string");
157-
printf(data); // GOOD [FALSE POSITIVE: Due to inaccurate dataflow killers]
157+
printf(data); // GOOD
158158
}
159159

160160
void fmt_with_assignment() {

‎cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/consts/NonConstantFormat.expected

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ edges
2323
| consts.cpp:106:13:106:19 | *call to varFunc | consts.cpp:107:9:107:10 | *v5 | provenance | |
2424
| consts.cpp:111:7:111:13 | *call to varFunc | consts.cpp:112:9:112:10 | *v6 | provenance | |
2525
| consts.cpp:139:13:139:16 | readString output argument | consts.cpp:140:9:140:11 | *v11 | provenance | |
26-
| consts.cpp:139:13:139:16 | readString output argument | consts.cpp:140:9:140:11 | v11 | provenance | |
2726
| consts.cpp:144:16:144:18 | readStringRef output argument | consts.cpp:145:9:145:11 | *v12 | provenance | |
28-
| consts.cpp:144:16:144:18 | readStringRef output argument | consts.cpp:145:9:145:11 | v12 | provenance | |
2927
nodes
3028
| consts.cpp:24:7:24:9 | **gv1 | semmle.label | **gv1 |
3129
| consts.cpp:29:7:29:25 | **nonConstFuncToArray | semmle.label | **nonConstFuncToArray |
@@ -47,13 +45,9 @@ nodes
4745
| consts.cpp:130:9:130:10 | *v9 | semmle.label | *v9 |
4846
| consts.cpp:135:9:135:11 | *v10 | semmle.label | *v10 |
4947
| consts.cpp:139:13:139:16 | readString output argument | semmle.label | readString output argument |
50-
| consts.cpp:139:13:139:16 | readString output argument | semmle.label | readString output argument |
5148
| consts.cpp:140:9:140:11 | *v11 | semmle.label | *v11 |
52-
| consts.cpp:140:9:140:11 | v11 | semmle.label | v11 |
53-
| consts.cpp:144:16:144:18 | readStringRef output argument | semmle.label | readStringRef output argument |
5449
| consts.cpp:144:16:144:18 | readStringRef output argument | semmle.label | readStringRef output argument |
5550
| consts.cpp:145:9:145:11 | *v12 | semmle.label | *v12 |
56-
| consts.cpp:145:9:145:11 | v12 | semmle.label | v12 |
5751
subpaths
5852
#select
5953
| consts.cpp:86:9:86:10 | *v1 | consts.cpp:85:7:85:8 | gets output argument | consts.cpp:86:9:86:10 | *v1 | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | consts.cpp:86:2:86:7 | call to printf | printf |
@@ -78,6 +72,4 @@ subpaths
7872
| consts.cpp:135:9:135:11 | *v10 | consts.cpp:85:7:85:8 | gets output argument | consts.cpp:135:9:135:11 | *v10 | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | consts.cpp:135:2:135:7 | call to printf | printf |
7973
| consts.cpp:135:9:135:11 | *v10 | consts.cpp:90:12:90:13 | gets output argument | consts.cpp:135:9:135:11 | *v10 | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | consts.cpp:135:2:135:7 | call to printf | printf |
8074
| consts.cpp:140:9:140:11 | *v11 | consts.cpp:139:13:139:16 | readString output argument | consts.cpp:140:9:140:11 | *v11 | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | consts.cpp:140:2:140:7 | call to printf | printf |
81-
| consts.cpp:140:9:140:11 | v11 | consts.cpp:139:13:139:16 | readString output argument | consts.cpp:140:9:140:11 | v11 | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | consts.cpp:140:2:140:7 | call to printf | printf |
8275
| consts.cpp:145:9:145:11 | *v12 | consts.cpp:144:16:144:18 | readStringRef output argument | consts.cpp:145:9:145:11 | *v12 | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | consts.cpp:145:2:145:7 | call to printf | printf |
83-
| consts.cpp:145:9:145:11 | v12 | consts.cpp:144:16:144:18 | readStringRef output argument | consts.cpp:145:9:145:11 | v12 | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | consts.cpp:145:2:145:7 | call to printf | printf |

0 commit comments

Comments
 (0)