Skip to content

Commit 9133a13

Browse files
MathiasVPMathiasVP
authored
Merge pull request #15628 from microsoft/cpp-non-constant-format-as-path-query
Cpp non constant format as path query
2 parents cb128da + a7547d5 commit 9133a13

File tree

4 files changed

+200
-43
lines changed

4 files changed

+200
-43
lines changed

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* type `const char*` it is still considered non-constant if the value is not coming from a string
99
* literal. For example, for a parameter with type `const char*` of an exported function that is
1010
* used as a format string, there is no way to ensure the originating value was a string literal.
11-
* @kind problem
11+
* @kind path-problem
1212
* @problem.severity recommendation
1313
* @security-severity 9.3
1414
* @precision high
@@ -26,6 +26,7 @@ import semmle.code.cpp.ir.dataflow.internal.ModelUtil
2626
import semmle.code.cpp.models.interfaces.DataFlow
2727
import semmle.code.cpp.models.interfaces.Taint
2828
import semmle.code.cpp.ir.IR
29+
import NonConstFlow::PathGraph
2930

3031
class UncalledFunction extends Function {
3132
UncalledFunction() {
@@ -68,7 +69,10 @@ predicate isNonConst(DataFlow::Node node) {
6869
// Parameters of uncalled functions that aren't const
6970
exists(UncalledFunction f, Parameter p |
7071
f.getAParameter() = p and
71-
p = node.asParameter()
72+
p = node.asParameter() and
73+
// Ignore main's argv parameter as it is already considered a `FlowSource`
74+
// not ignoring it will result in path redundancies
75+
(f.getName() = "main" implies p != f.getParameter(1))
7276
)
7377
or
7478
// Consider as an input any out arg of a function or a function's return where the function is not:
@@ -127,11 +131,13 @@ module NonConstFlowConfig implements DataFlow::ConfigSig {
127131

128132
module NonConstFlow = TaintTracking::Global<NonConstFlowConfig>;
129133

130-
from FormattingFunctionCall call, Expr formatString, DataFlow::Node sink
134+
from
135+
FormattingFunctionCall call, Expr formatString, NonConstFlow::PathNode sink,
136+
NonConstFlow::PathNode source
131137
where
138+
isSinkImpl(sink.getNode(), formatString) and
132139
call.getArgument(call.getFormatParameterIndex()) = formatString and
133-
NonConstFlow::flowTo(sink) and
134-
isSinkImpl(sink, formatString)
135-
select formatString,
136-
"The format string argument to " + call.getTarget().getName() +
137-
" should be constant to prevent security issues and other potential errors."
140+
NonConstFlow::flowPath(source, sink)
141+
select sink.getNode(), source, sink,
142+
"The format string argument to $@ has a source which cannot be " +
143+
"verified to originate from a string literal.", call, call.getTarget().getName()

‎cpp/ql/src/change-notes/2024-02-29-non-constant-format-path-query.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The "non-constant format string" query (`cpp/non-constant-format`) has been converted to a `path-problem` query.

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

Lines changed: 99 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,99 @@
1-
| NonConstantFormat.c:30:10:30:16 | access to array | The format string argument to printf should be constant to prevent security issues and other potential errors. |
2-
| NonConstantFormat.c:41:9:41:27 | call to any_random_function | The format string argument to printf should be constant to prevent security issues and other potential errors. |
3-
| NonConstantFormat.c:45:9:45:48 | call to gettext | The format string argument to printf should be constant to prevent security issues and other potential errors. |
4-
| nested.cpp:21:23:21:26 | fmt0 | The format string argument to snprintf should be constant to prevent security issues and other potential errors. |
5-
| nested.cpp:79:32:79:38 | call to get_fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
6-
| nested.cpp:87:18:87:20 | fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
7-
| test.cpp:51:10:51:21 | call to make_message | The format string argument to printf should be constant to prevent security issues and other potential errors. |
8-
| test.cpp:130:20:130:26 | access to array | The format string argument to sprintf should be constant to prevent security issues and other potential errors. |
9-
| test.cpp:157:12:157:15 | data | The format string argument to printf should be constant to prevent security issues and other potential errors. |
10-
| test.cpp:170:12:170:14 | res | The format string argument to printf should be constant to prevent security issues and other potential errors. |
11-
| test.cpp:195:31:195:33 | str | The format string argument to StringCchPrintfW should be constant to prevent security issues and other potential errors. |
12-
| test.cpp:197:11:197:14 | wstr | The format string argument to wprintf should be constant to prevent security issues and other potential errors. |
13-
| test.cpp:205:12:205:20 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
14-
| test.cpp:206:12:206:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
15-
| test.cpp:211:12:211:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
16-
| test.cpp:217:12:217:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
17-
| test.cpp:223:12:223:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
18-
| test.cpp:228:12:228:18 | ++ ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
19-
| test.cpp:235:12:235:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
20-
| test.cpp:242:12:242:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
21-
| test.cpp:247:12:247:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
1+
edges
2+
| NonConstantFormat.c:28:27:28:30 | **argv | NonConstantFormat.c:30:10:30:16 | *access to array | provenance | |
3+
| NonConstantFormat.c:45:11:45:47 | *call to any_random_function | NonConstantFormat.c:45:9:45:48 | *call to gettext | provenance | |
4+
| nested.cpp:19:29:19:32 | *fmt0 | nested.cpp:21:23:21:26 | *fmt0 | provenance | |
5+
| nested.cpp:27:32:27:34 | *fmt | nested.cpp:28:16:28:18 | *fmt | provenance | |
6+
| nested.cpp:28:16:28:18 | *fmt | nested.cpp:19:29:19:32 | *fmt0 | provenance | |
7+
| nested.cpp:34:37:34:39 | *fmt | nested.cpp:35:19:35:21 | *fmt | provenance | |
8+
| nested.cpp:35:19:35:21 | *fmt | nested.cpp:27:32:27:34 | *fmt | provenance | |
9+
| nested.cpp:42:24:42:34 | *call to ext_fmt_str | nested.cpp:34:37:34:39 | *fmt | provenance | |
10+
| 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 | |
13+
| 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 | |
20+
| test.cpp:204:25:204:36 | *call to get_string | test.cpp:205:12:205:20 | *... + ... | provenance | |
21+
| test.cpp:204:25:204:36 | *call to get_string | test.cpp:206:12:206:16 | *hello | provenance | |
22+
| test.cpp:209:25:209:36 | *call to get_string | test.cpp:211:12:211:16 | *hello | provenance | |
23+
| test.cpp:215:25:215:36 | *call to get_string | test.cpp:217:12:217:16 | *hello | provenance | |
24+
| test.cpp:221:25:221:36 | *call to get_string | test.cpp:223:12:223:16 | *hello | provenance | |
25+
| test.cpp:227:25:227:36 | *call to get_string | test.cpp:228:12:228:18 | *++ ... | provenance | |
26+
| test.cpp:232:25:232:36 | *call to get_string | test.cpp:235:12:235:16 | *hello | provenance | |
27+
| test.cpp:239:25:239:36 | *call to get_string | test.cpp:242:12:242:16 | *hello | provenance | |
28+
| test.cpp:245:25:245:36 | *call to get_string | test.cpp:247:12:247:16 | *hello | provenance | |
29+
nodes
30+
| NonConstantFormat.c:28:27:28:30 | **argv | semmle.label | **argv |
31+
| NonConstantFormat.c:30:10:30:16 | *access to array | semmle.label | *access to array |
32+
| NonConstantFormat.c:41:9:41:45 | *call to any_random_function | semmle.label | *call to any_random_function |
33+
| NonConstantFormat.c:45:9:45:48 | *call to gettext | semmle.label | *call to gettext |
34+
| NonConstantFormat.c:45:11:45:47 | *call to any_random_function | semmle.label | *call to any_random_function |
35+
| nested.cpp:19:29:19:32 | *fmt0 | semmle.label | *fmt0 |
36+
| nested.cpp:21:23:21:26 | *fmt0 | semmle.label | *fmt0 |
37+
| nested.cpp:27:32:27:34 | *fmt | semmle.label | *fmt |
38+
| nested.cpp:28:16:28:18 | *fmt | semmle.label | *fmt |
39+
| nested.cpp:34:37:34:39 | *fmt | semmle.label | *fmt |
40+
| nested.cpp:35:19:35:21 | *fmt | semmle.label | *fmt |
41+
| nested.cpp:42:24:42:34 | *call to ext_fmt_str | semmle.label | *call to ext_fmt_str |
42+
| nested.cpp:79:32:79:38 | *call to get_fmt | semmle.label | *call to get_fmt |
43+
| nested.cpp:86:19:86:46 | *call to __builtin_alloca | semmle.label | *call to __builtin_alloca |
44+
| 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 |
48+
| 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 | ... - ... |
51+
| 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 |
55+
| 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 |
58+
| test.cpp:197:11:197:14 | *wstr | semmle.label | *wstr |
59+
| test.cpp:204:25:204:36 | *call to get_string | semmle.label | *call to get_string |
60+
| test.cpp:205:12:205:20 | *... + ... | semmle.label | *... + ... |
61+
| test.cpp:206:12:206:16 | *hello | semmle.label | *hello |
62+
| test.cpp:209:25:209:36 | *call to get_string | semmle.label | *call to get_string |
63+
| test.cpp:211:12:211:16 | *hello | semmle.label | *hello |
64+
| test.cpp:215:25:215:36 | *call to get_string | semmle.label | *call to get_string |
65+
| test.cpp:217:12:217:16 | *hello | semmle.label | *hello |
66+
| test.cpp:221:25:221:36 | *call to get_string | semmle.label | *call to get_string |
67+
| test.cpp:223:12:223:16 | *hello | semmle.label | *hello |
68+
| test.cpp:227:25:227:36 | *call to get_string | semmle.label | *call to get_string |
69+
| test.cpp:228:12:228:18 | *++ ... | semmle.label | *++ ... |
70+
| test.cpp:232:25:232:36 | *call to get_string | semmle.label | *call to get_string |
71+
| test.cpp:235:12:235:16 | *hello | semmle.label | *hello |
72+
| test.cpp:239:25:239:36 | *call to get_string | semmle.label | *call to get_string |
73+
| test.cpp:242:12:242:16 | *hello | semmle.label | *hello |
74+
| test.cpp:245:25:245:36 | *call to get_string | semmle.label | *call to get_string |
75+
| test.cpp:247:12:247:16 | *hello | semmle.label | *hello |
76+
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 |
78+
#select
79+
| 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 |
80+
| 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 |
81+
| 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 |
82+
| 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 |
83+
| 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 |
84+
| 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 |
86+
| 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 |
91+
| 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 |
92+
| 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 |
93+
| 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 |
94+
| test.cpp:217:12:217:16 | *hello | test.cpp:215:25:215:36 | *call to get_string | test.cpp:217:12:217:16 | *hello | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:217:5:217:10 | call to printf | printf |
95+
| test.cpp:223:12:223:16 | *hello | test.cpp:221:25:221:36 | *call to get_string | test.cpp:223:12:223:16 | *hello | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:223:5:223:10 | call to printf | printf |
96+
| test.cpp:228:12:228:18 | *++ ... | test.cpp:227:25:227:36 | *call to get_string | test.cpp:228:12:228:18 | *++ ... | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:228:5:228:10 | call to printf | printf |
97+
| test.cpp:235:12:235:16 | *hello | test.cpp:232:25:232:36 | *call to get_string | test.cpp:235:12:235:16 | *hello | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:235:5:235:10 | call to printf | printf |
98+
| test.cpp:242:12:242:16 | *hello | test.cpp:239:25:239:36 | *call to get_string | test.cpp:242:12:242:16 | *hello | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:242:5:242:10 | call to printf | printf |
99+
| test.cpp:247:12:247:16 | *hello | test.cpp:245:25:245:36 | *call to get_string | test.cpp:247:12:247:16 | *hello | The format string argument to $@ has a source which cannot be verified to originate from a string literal. | test.cpp:247:5:247:10 | call to printf | printf |

0 commit comments

Comments
 (0)