This patch adds two new analyzer warnings for misuse of mkstemp(3):
-Wanalyzer-mkstemp-of-string-literal warns when a string literal is
passed to mkstemp. Since mkstemp modifies its argument in place,
passing a string literal is undefined behavior (SEI CERT C rule
STR30-C). The diagnostic suggests using a writable character array
instead.
-Wanalyzer-mkstemp-missing-suffix warns when the template argument
does not end with the required "XXXXXX" suffix. This addresses PR
analyzer/105890.
Both warnings are enabled by default under -fanalyzer.
The checks are in the analyzer rather than -Wformat because mkstemp
does not use a format attribute. Placing the checks in the analyzer
could also allow interprocedural analysis in the future, once the
analyzer can fully track string contents across function calls.
Bootstrapped and tested on x86_64-pc-linux-gnu.
gcc/analyzer/ChangeLog:
PR analyzer/105890
* analyzer.opt: Add -Wanalyzer-mkstemp-missing-suffix and
-Wanalyzer-mkstemp-of-string-literal.
* analyzer.opt.urls: Add URL entries for the new warnings.
* kf.cc (class mkstemp_of_string_literal): New diagnostic class
for mkstemp called on a string literal.
(class mkstemp_missing_suffix): New diagnostic class for mkstemp
called with a template missing the "XXXXXX" suffix.
(class kf_mkstemp): New known_function handler for mkstemp.
(register_known_functions): Register kf_mkstemp.
gcc/ChangeLog:
PR analyzer/105890
* doc/invoke.texi: Add -Wanalyzer-mkstemp-missing-suffix and
-Wanalyzer-mkstemp-of-string-literal.
gcc/testsuite/ChangeLog:
PR analyzer/105890
* gcc.dg/analyzer/mkstemp-1.c: New test.
Signed-off-by: Tomas Ortin Fernandez (quanrong) <[email protected]>
---
gcc/analyzer/analyzer.opt | 8 +
gcc/analyzer/analyzer.opt.urls | 6 +
gcc/analyzer/kf.cc | 201 ++++++++++++++++++++++
gcc/doc/invoke.texi | 25 +++
gcc/testsuite/gcc.dg/analyzer/mkstemp-1.c | 95 ++++++++++
5 files changed, 335 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/mkstemp-1.c
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 3c5dd0849c6..8b683b4cb6a 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -182,6 +182,14 @@ Wanalyzer-null-dereference
Common Var(warn_analyzer_null_dereference) Init(1) Warning
Warn about code paths in which a NULL pointer is dereferenced.
+Wanalyzer-mkstemp-missing-suffix
+Common Var(warn_analyzer_mkstemp_missing_suffix) Init(1) Warning
+Warn about code paths in which mkstemp is called with a template not
ending in \"XXXXXX\".
+
+Wanalyzer-mkstemp-of-string-literal
+Common Var(warn_analyzer_mkstemp_of_string_literal) Init(1) Warning
+Warn about code paths in which a string literal is passed to mkstemp.
+
Wanalyzer-putenv-of-auto-var
Common Var(warn_analyzer_putenv_of_auto_var) Init(1) Warning
Warn about code paths in which an on-stack buffer is passed to putenv.
diff --git a/gcc/analyzer/analyzer.opt.urls b/gcc/analyzer/analyzer.opt.urls
index 1a698f9c6d9..d98174a00d6 100644
--- a/gcc/analyzer/analyzer.opt.urls
+++ b/gcc/analyzer/analyzer.opt.urls
@@ -84,6 +84,12 @@
UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-null-argument)
Wanalyzer-null-dereference
UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-null-dereference)
+Wanalyzer-mkstemp-missing-suffix
+UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-mkstemp-missing-suffix)
+
+Wanalyzer-mkstemp-of-string-literal
+UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-mkstemp-of-string-literal)
+
Wanalyzer-putenv-of-auto-var
UrlSuffix(gcc/Static-Analyzer-Options.html#index-Wanalyzer-putenv-of-auto-var)
diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc
index b1ccbd6584a..6ecdf5c4006 100644
--- a/gcc/analyzer/kf.cc
+++ b/gcc/analyzer/kf.cc
@@ -747,6 +747,206 @@ kf_memset::impl_call_pre (const call_details &cd)
const
cd.maybe_set_lhs (dest_sval);
}
+/* A subclass of pending_diagnostic for complaining about 'mkstemp'
+ called on a string literal. */
+
+class mkstemp_of_string_literal : public undefined_function_behavior
+{
+public:
+ mkstemp_of_string_literal (const call_details &cd)
+ : undefined_function_behavior (cd)
+ {
+ }
+
+ int
+ get_controlling_option () const final override
+ {
+ return OPT_Wanalyzer_mkstemp_of_string_literal;
+ }
+
+ bool
+ emit (diagnostic_emission_context &ctxt) final override
+ {
+ auto_diagnostic_group d;
+
+ /* SEI CERT C Coding Standard: "STR30-C. Do not attempt to modify
string
+ literals. */
+ diagnostics::metadata::precanned_rule rule (
+ "STR30-C", "https://wiki.sei.cmu.edu/confluence/x/VtYxBQ";);
+ ctxt.add_rule (rule);
+
+ bool warned = ctxt.warn ("%qE on a string literal",
get_callee_fndecl ());
+ if (warned)
+ inform (ctxt.get_location (),
+ "use a writable character array as the template argument,"
+ " e.g. %<char tmpl[] = \"/tmp/fooXXXXXX\"%>");
+ return warned;
+ }
+
+ bool
+ describe_final_event (pretty_printer &pp,
+ const evdesc::final_event &) final override
+ {
+ pp_printf (&pp, "%qE on a string literal", get_callee_fndecl ());
+ return true;
+ }
+};
+
+/* A subclass of pending_diagnostic for complaining about 'mkstemp'
+ called with a template that does not end with "XXXXXX". */
+
+class mkstemp_missing_suffix
+ : public pending_diagnostic_subclass<mkstemp_missing_suffix>
+{
+public:
+ mkstemp_missing_suffix (const call_details &cd)
+ : m_call_stmt (cd.get_call_stmt ()), m_fndecl
(cd.get_fndecl_for_call ())
+ {
+ gcc_assert (m_fndecl);
+ }
+
+ const char *
+ get_kind () const final override
+ {
+ return "mkstemp_missing_suffix";
+ }
+
+ bool
+ operator== (const mkstemp_missing_suffix &other) const
+ {
+ return &m_call_stmt == &other.m_call_stmt;
+ }
+
+ int
+ get_controlling_option () const final override
+ {
+ return OPT_Wanalyzer_mkstemp_missing_suffix;
+ }
+
+ bool
+ emit (diagnostic_emission_context &ctxt) final override
+ {
+ return ctxt.warn ("%qE template string does not end with %qs",
m_fndecl,
+ "XXXXXX");
+ }
+
+ bool
+ describe_final_event (pretty_printer &pp,
+ const evdesc::final_event &) final override
+ {
+ pp_printf (&pp, "%qE template string does not end with %qs", m_fndecl,
+ "XXXXXX");
+ return true;
+ }
+
+private:
+ const gimple &m_call_stmt;
+ tree m_fndecl; // non-NULL
+};
+
+/* Handler for calls to "mkstemp":
+
+ int mkstemp(char *template);
+
+ The template must not be a string constant, and its last six
+ characters must be "XXXXXX". */
+
+class kf_mkstemp : public known_function
+{
+public:
+ bool
+ matches_call_types_p (const call_details &cd) const final override
+ {
+ return (cd.num_args () == 1 && cd.arg_is_pointer_p (0));
+ }
+
+ void
+ impl_call_pre (const call_details &cd) const final override
+ {
+ if (cd.get_ctxt ())
+ check_template (cd);
+
+ cd.set_any_lhs_with_defaults ();
+ }
+
+private:
+ static void
+ check_template (const call_details &cd)
+ {
+ region_model_context *ctxt = cd.get_ctxt ();
+ const svalue *ptr_sval = cd.get_arg_svalue (0);
+ region_model *model = cd.get_model ();
+
+ const svalue *strlen_sval
+ = model->check_for_null_terminated_string_arg (cd, 0, false,
nullptr);
+ if (!strlen_sval)
+ return;
+
+ if (cd.get_arg_string_literal (0))
+ {
+ ctxt->warn (std::make_unique<mkstemp_of_string_literal> (cd));
+ }
+ else if (check_suffix (cd, ptr_sval, strlen_sval).is_false ())
+ {
+ ctxt->warn (std::make_unique<mkstemp_missing_suffix> (cd));
+ }
+ }
+
+ /* Return true if the template ends with "XXXXXX", false if it
+ definitely does not, or unknown if we can't determine. */
+ static tristate
+ check_suffix (const call_details &cd, const svalue *ptr_sval,
+ const svalue *strlen_sval)
+ {
+ region_model *model = cd.get_model ();
+
+ const constant_svalue *len_cst =
strlen_sval->dyn_cast_constant_svalue ();
+ if (!len_cst)
+ return tristate::TS_UNKNOWN;
+
+ byte_offset_t len = TREE_INT_CST_LOW (len_cst->get_constant ());
+ if (len < 6)
+ return tristate::TS_FALSE;
+
+ tree arg_tree = cd.get_arg_tree (0);
+ const region *reg
+ = model->deref_rvalue (ptr_sval, arg_tree, cd.get_ctxt ());
+
+ /* Find the byte offset of the pointed-to region. */
+ region_offset reg_offset = reg->get_offset (cd.get_manager ());
+ if (reg_offset.symbolic_p ())
+ return tristate::TS_UNKNOWN;
+ byte_offset_t ptr_byte_offset;
+ if (!reg_offset.get_concrete_byte_offset (&ptr_byte_offset))
+ return tristate::TS_UNKNOWN;
+
+ const region *base_reg = reg->get_base_region ();
+ const svalue *base_sval
+ = model->get_store_value (base_reg, cd.get_ctxt ());
+
+ const constant_svalue *cst_sval =
base_sval->dyn_cast_constant_svalue ();
+ if (!cst_sval)
+ return tristate::TS_UNKNOWN;
+
+ tree cst = cst_sval->get_constant ();
+ if (TREE_CODE (cst) != STRING_CST)
+ return tristate::TS_UNKNOWN;
+
+ HOST_WIDE_INT str_len = len.to_shwi ();
+ HOST_WIDE_INT start = ptr_byte_offset.to_shwi ();
+
+ /* Ensure the range [start, start + str_len] fits. */
+ if (1 + start + str_len > TREE_STRING_LENGTH (cst))
+ return tristate::TS_UNKNOWN;
+
+ if (memcmp (TREE_STRING_POINTER (cst) + start + str_len - 6,
"XXXXXX", 6)
+ != 0)
+ return tristate::TS_FALSE;
+
+ return tristate::TS_TRUE;
+ }
+};
+
/* A subclass of pending_diagnostic for complaining about 'putenv'
called on an auto var. */
@@ -2346,6 +2546,7 @@ register_known_functions (known_function_manager &kfm,
/* Known POSIX functions, and some non-standard extensions. */
{
kfm.add ("fopen", std::make_unique<kf_fopen> ());
+ kfm.add ("mkstemp", std::make_unique<kf_mkstemp> ());
kfm.add ("putenv", std::make_unique<kf_putenv> ());
kfm.add ("strtok", std::make_unique<kf_strtok> (rmm));
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4b820015215..7fac910dd0f 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -527,6 +527,8 @@ Objective-C and Objective-C++ Dialects}.
-Wno-analyzer-jump-through-null
-Wno-analyzer-malloc-leak
-Wno-analyzer-mismatching-deallocation
+-Wno-analyzer-mkstemp-of-string-literal
+-Wno-analyzer-mkstemp-missing-suffix
-Wno-analyzer-null-argument
-Wno-analyzer-null-dereference
-Wno-analyzer-out-of-bounds
@@ -11559,6 +11561,8 @@ Enabling this option effectively enables the
following warnings:
-Wanalyzer-jump-through-null
-Wanalyzer-malloc-leak
-Wanalyzer-mismatching-deallocation
+-Wanalyzer-mkstemp-of-string-literal
+-Wanalyzer-mkstemp-missing-suffix
-Wanalyzer-null-argument
-Wanalyzer-null-dereference
-Wanalyzer-out-of-bounds
@@ -11935,6 +11939,27 @@ or a function marked with attribute @code{malloc}.
See @uref{https://cwe.mitre.org/data/definitions/401.html, CWE-401:
Missing Release of Memory after Effective Lifetime}.
+@opindex Wanalyzer-mkstemp-missing-suffix
+@opindex Wno-analyzer-mkstemp-missing-suffix
+@item -Wno-analyzer-mkstemp-missing-suffix
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-mkstemp-missing-suffix} to disable it.
+
+This diagnostic warns for paths through the code in which the template
+string passed to @code{mkstemp} does not end with @samp{XXXXXX}.
+
+@opindex Wanalyzer-mkstemp-of-string-literal
+@opindex Wno-analyzer-mkstemp-of-string-literal
+@item -Wno-analyzer-mkstemp-of-string-literal
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-mkstemp-of-string-literal} to disable it.
+
+This diagnostic warns for paths through the code in which @code{mkstemp}
+is called on a string literal. Since @code{mkstemp} modifies its
+argument in place, passing a string literal leads to undefined behavior.
+
+See @uref{https://wiki.sei.cmu.edu/confluence/x/VtYxBQ, STR30-C. Do not
attempt to modify string literals}.
+
@opindex Wanalyzer-mismatching-deallocation
@opindex Wno-analyzer-mismatching-deallocation
@item -Wno-analyzer-mismatching-deallocation
diff --git a/gcc/testsuite/gcc.dg/analyzer/mkstemp-1.c
b/gcc/testsuite/gcc.dg/analyzer/mkstemp-1.c
new file mode 100644
index 00000000000..2eda175f29f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/mkstemp-1.c
@@ -0,0 +1,95 @@
+/* { dg-additional-options "-Wno-analyzer-null-argument" } */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+extern void populate (char *buf);
+
+void test_passthrough (char *s)
+{
+ mkstemp (s);
+}
+
+void test_string_literal_correct_suffix (void)
+{
+ mkstemp ("/tmp/fooXXXXXX"); /* { dg-warning "'mkstemp' on a string
literal \\\[STR30-C\\\]" } */
+ /* { dg-message "use a writable character array" "fix suggestion" {
target *-*-* } .-1 } */
+}
+
+void test_string_literal_missing_suffix (void)
+{
+ mkstemp ("/tmp/foo"); /* { dg-warning "'mkstemp' on a string literal
\\\[STR30-C\\\]" } */
+}
+
+void test_string_literal_empty (void)
+{
+ mkstemp (""); /* { dg-warning "'mkstemp' on a string literal
\\\[STR30-C\\\]" } */
+}
+
+void test_correct (void)
+{
+ char tmpl[] = "/tmp/fooXXXXXX";
+ mkstemp (tmpl);
+}
+
+void test_correct_minimal (void)
+{
+ char tmpl[] = "XXXXXX";
+ mkstemp (tmpl);
+}
+
+void test_correct_offset_into_buffer (void)
+{
+ char buf[] = "/tmp/XXXXXX";
+ /* Suffix is still correct from the pointer's perspective. */
+ mkstemp (buf + 5);
+}
+
+void test_missing_suffix_offset_into_buffer (void)
+{
+ char buf[] = "/tmp/XXXXXX";
+ /* Suffix is incorrect from the pointer's perspective. */
+ mkstemp (buf + 6); /* { dg-warning "'mkstemp' template string does
not end with 'XXXXXX'" } */
+}
+
+void test_missing_suffix (void)
+{
+ char tmpl[] = "/tmp/foo";
+ mkstemp (tmpl); /* { dg-warning "'mkstemp' template string does not
end with 'XXXXXX'" } */
+}
+
+void test_too_short (void)
+{
+ char tmpl[] = "XXX";
+ mkstemp (tmpl); /* { dg-warning "'mkstemp' template string does not
end with 'XXXXXX'" } */
+}
+
+void test_empty_buffer (void)
+{
+ char tmpl[] = "";
+ mkstemp (tmpl); /* { dg-warning "'mkstemp' template string does not
end with 'XXXXXX'" } */
+}
+
+void test_partial_suffix (void)
+{
+ char tmpl[] = "/tmp/fooXXXXX_";
+ mkstemp (tmpl); /* { dg-warning "'mkstemp' template string does not
end with 'XXXXXX'" } */
+}
+
+void test_five_xs (void)
+{
+ char tmpl[] = "/tmp/fooXXXXX";
+ mkstemp (tmpl); /* { dg-warning "'mkstemp' template string does not
end with 'XXXXXX'" } */
+}
+
+void test_populated_buf (void)
+{
+ char tmpl[20];
+ populate (tmpl);
+ mkstemp (tmpl);
+}
+
+void test_NULL (void)
+{
+ mkstemp (NULL); /* possibly -Wanalyzer-null-argument */
+}
--
2.52.0
