The non-constant format query is now a path query. Minor changes to the output alert to be more precise on what is being alerted. Minor changes to the query itself to avoid redundancies with argv.

This commit is contained in:
Benjamin Rodes
2024-02-15 12:14:52 -05:00
parent 9e50fc6893
commit d6b0746b30
2 changed files with 114 additions and 29 deletions

View File

@@ -12,7 +12,7 @@
* via `c_str()`, which returns a `const char*`, regardless if the original string was a string literal or not.
* The query does not consider uninitialized variables as non-constant sources. Uninitialized
* variables are a separate vulnerability concern and should be addressed by a separate query.
* @kind problem
* @kind path-problem
* @problem.severity recommendation
* @security-severity 9.3
* @precision high
@@ -30,6 +30,7 @@ import semmle.code.cpp.ir.dataflow.internal.ModelUtil
import semmle.code.cpp.models.interfaces.DataFlow
import semmle.code.cpp.models.interfaces.Taint
import semmle.code.cpp.ir.IR
import NonConstFlow::PathGraph
class UncalledFunction extends Function {
UncalledFunction() {
@@ -72,7 +73,11 @@ predicate isNonConst(DataFlow::Node node) {
// Parameters of uncalled functions that aren't const
exists(UncalledFunction f, Parameter p |
f.getAParameter() = p and
p = node.asParameter()
p = node.asParameter() and
// Exclude main in this instance since that should have its own defined FlowSource
// and including main would likely result in redundancy.
// Note, argc is not a flow source, so only filter out argv
(p.getFunction().getName() = "main" implies not p.getName() = "argv")
)
or
// Consider as an input any out arg of a function or a function's return where the function is not:
@@ -127,11 +132,13 @@ module NonConstFlowConfig implements DataFlow::ConfigSig {
module NonConstFlow = TaintTracking::Global<NonConstFlowConfig>;
from FormattingFunctionCall call, Expr formatString, DataFlow::Node sink
from
FormattingFunctionCall call, Expr formatString, NonConstFlow::PathNode sink,
NonConstFlow::PathNode source
where
isSinkImpl(sink.getNode(), formatString) and
call.getArgument(call.getFormatParameterIndex()) = formatString and
NonConstFlow::flowTo(sink) and
isSinkImpl(sink, formatString)
select formatString,
"The format string argument to " + call.getTarget().getName() +
" should be constant to prevent security issues and other potential errors."
NonConstFlow::flowPath(source, sink)
select sink.getNode(), source, sink,
"The format string argument to $@ has a source which cannot be " +
"verified to originate from a string literal.", call, call.getTarget().getName()

View File

@@ -1,21 +1,99 @@
| 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. |
| 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. |
| 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. |
| nested.cpp:21:23:21:26 | fmt0 | The format string argument to snprintf should be constant to prevent security issues and other potential errors. |
| 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. |
| nested.cpp:87:18:87:20 | fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
| 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. |
| 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. |
| test.cpp:157:12:157:15 | data | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:170:12:170:14 | res | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:195:31:195:33 | str | The format string argument to StringCchPrintfW should be constant to prevent security issues and other potential errors. |
| test.cpp:197:11:197:14 | wstr | The format string argument to wprintf should be constant to prevent security issues and other potential errors. |
| test.cpp:205:12:205:20 | ... + ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:206:12:206:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:211:12:211:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:217:12:217:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:223:12:223:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:228:12:228:18 | ++ ... | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:235:12:235:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:242:12:242:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
| test.cpp:247:12:247:16 | hello | The format string argument to printf should be constant to prevent security issues and other potential errors. |
edges
| NonConstantFormat.c:28:27:28:30 | **argv | NonConstantFormat.c:30:10:30:16 | *access to array |
| NonConstantFormat.c:45:11:45:47 | *call to any_random_function | NonConstantFormat.c:45:9:45:48 | *call to gettext |
| nested.cpp:19:29:19:32 | *fmt0 | nested.cpp:21:23:21:26 | *fmt0 |
| nested.cpp:27:32:27:34 | *fmt | nested.cpp:28:16:28:18 | *fmt |
| nested.cpp:28:16:28:18 | *fmt | nested.cpp:19:29:19:32 | *fmt0 |
| nested.cpp:34:37:34:39 | *fmt | nested.cpp:35:19:35:21 | *fmt |
| nested.cpp:35:19:35:21 | *fmt | nested.cpp:27:32:27:34 | *fmt |
| nested.cpp:42:24:42:34 | *call to ext_fmt_str | nested.cpp:34:37:34:39 | *fmt |
| nested.cpp:86:19:86:46 | *call to __builtin_alloca | nested.cpp:87:18:87:20 | *fmt |
| test.cpp:27:39:27:39 | n | test.cpp:27:13:27:24 | **make_message |
| test.cpp:46:14:46:17 | argc | test.cpp:51:23:51:30 | ... - ... |
| test.cpp:46:27:46:30 | **argv | test.cpp:130:20:130:26 | *access to array |
| test.cpp:51:23:51:30 | ... - ... | test.cpp:27:39:27:39 | n |
| test.cpp:51:23:51:30 | ... - ... | test.cpp:51:10:51:21 | *call to make_message |
| test.cpp:155:27:155:30 | data | test.cpp:157:12:157:15 | data |
| test.cpp:167:31:167:34 | data | test.cpp:170:12:170:14 | *res |
| test.cpp:193:32:193:34 | str | test.cpp:195:31:195:33 | str |
| test.cpp:193:32:193:34 | str | test.cpp:197:11:197:14 | *wstr |
| test.cpp:204:25:204:36 | *call to get_string | test.cpp:205:12:205:20 | *... + ... |
| test.cpp:204:25:204:36 | *call to get_string | test.cpp:206:12:206:16 | *hello |
| test.cpp:209:25:209:36 | *call to get_string | test.cpp:211:12:211:16 | *hello |
| test.cpp:215:25:215:36 | *call to get_string | test.cpp:217:12:217:16 | *hello |
| test.cpp:221:25:221:36 | *call to get_string | test.cpp:223:12:223:16 | *hello |
| test.cpp:227:25:227:36 | *call to get_string | test.cpp:228:12:228:18 | *++ ... |
| test.cpp:232:25:232:36 | *call to get_string | test.cpp:235:12:235:16 | *hello |
| test.cpp:239:25:239:36 | *call to get_string | test.cpp:242:12:242:16 | *hello |
| test.cpp:245:25:245:36 | *call to get_string | test.cpp:247:12:247:16 | *hello |
nodes
| NonConstantFormat.c:28:27:28:30 | **argv | semmle.label | **argv |
| NonConstantFormat.c:30:10:30:16 | *access to array | semmle.label | *access to array |
| NonConstantFormat.c:41:9:41:45 | *call to any_random_function | semmle.label | *call to any_random_function |
| NonConstantFormat.c:45:9:45:48 | *call to gettext | semmle.label | *call to gettext |
| NonConstantFormat.c:45:11:45:47 | *call to any_random_function | semmle.label | *call to any_random_function |
| nested.cpp:19:29:19:32 | *fmt0 | semmle.label | *fmt0 |
| nested.cpp:21:23:21:26 | *fmt0 | semmle.label | *fmt0 |
| nested.cpp:27:32:27:34 | *fmt | semmle.label | *fmt |
| nested.cpp:28:16:28:18 | *fmt | semmle.label | *fmt |
| nested.cpp:34:37:34:39 | *fmt | semmle.label | *fmt |
| nested.cpp:35:19:35:21 | *fmt | semmle.label | *fmt |
| nested.cpp:42:24:42:34 | *call to ext_fmt_str | semmle.label | *call to ext_fmt_str |
| nested.cpp:79:32:79:38 | *call to get_fmt | semmle.label | *call to get_fmt |
| nested.cpp:86:19:86:46 | *call to __builtin_alloca | semmle.label | *call to __builtin_alloca |
| nested.cpp:87:18:87:20 | *fmt | semmle.label | *fmt |
| test.cpp:27:13:27:24 | **make_message | semmle.label | **make_message |
| test.cpp:27:39:27:39 | n | semmle.label | n |
| test.cpp:46:14:46:17 | argc | semmle.label | argc |
| test.cpp:46:27:46:30 | **argv | semmle.label | **argv |
| test.cpp:51:10:51:21 | *call to make_message | semmle.label | *call to make_message |
| test.cpp:51:23:51:30 | ... - ... | semmle.label | ... - ... |
| test.cpp:130:20:130:26 | *access to array | semmle.label | *access to array |
| test.cpp:155:27:155:30 | data | semmle.label | data |
| test.cpp:157:12:157:15 | data | semmle.label | data |
| test.cpp:167:31:167:34 | data | semmle.label | data |
| test.cpp:170:12:170:14 | *res | semmle.label | *res |
| test.cpp:193:32:193:34 | str | semmle.label | str |
| test.cpp:195:31:195:33 | str | semmle.label | str |
| test.cpp:197:11:197:14 | *wstr | semmle.label | *wstr |
| test.cpp:204:25:204:36 | *call to get_string | semmle.label | *call to get_string |
| test.cpp:205:12:205:20 | *... + ... | semmle.label | *... + ... |
| test.cpp:206:12:206:16 | *hello | semmle.label | *hello |
| test.cpp:209:25:209:36 | *call to get_string | semmle.label | *call to get_string |
| test.cpp:211:12:211:16 | *hello | semmle.label | *hello |
| test.cpp:215:25:215:36 | *call to get_string | semmle.label | *call to get_string |
| test.cpp:217:12:217:16 | *hello | semmle.label | *hello |
| test.cpp:221:25:221:36 | *call to get_string | semmle.label | *call to get_string |
| test.cpp:223:12:223:16 | *hello | semmle.label | *hello |
| test.cpp:227:25:227:36 | *call to get_string | semmle.label | *call to get_string |
| test.cpp:228:12:228:18 | *++ ... | semmle.label | *++ ... |
| test.cpp:232:25:232:36 | *call to get_string | semmle.label | *call to get_string |
| test.cpp:235:12:235:16 | *hello | semmle.label | *hello |
| test.cpp:239:25:239:36 | *call to get_string | semmle.label | *call to get_string |
| test.cpp:242:12:242:16 | *hello | semmle.label | *hello |
| test.cpp:245:25:245:36 | *call to get_string | semmle.label | *call to get_string |
| test.cpp:247:12:247:16 | *hello | semmle.label | *hello |
subpaths
| 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 |
#select
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |