public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-1881] analyzer: new warning: -Wanalyzer-putenv-of-auto-var [PR105893]
@ 2022-07-28 21:25 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2022-07-28 21:25 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:872693eebb6b88f4b6a2767727a9565d05172768
commit r13-1881-g872693eebb6b88f4b6a2767727a9565d05172768
Author: David Malcolm <dmalcolm@redhat.com>
Date: Thu Jul 28 17:21:29 2022 -0400
analyzer: new warning: -Wanalyzer-putenv-of-auto-var [PR105893]
This patch implements a new -fanalyzer warning:
-Wanalyzer-putenv-of-auto-var
which complains about stack pointers passed to putenv(3) calls, as
per SEI CERT C Coding Standard rule POS34-C ("Do not call putenv() with
a pointer to an automatic variable as the argument").
For example, given:
#include <stdio.h>
#include <stdlib.h>
void test_arr (void)
{
char arr[] = "NAME=VALUE";
putenv (arr);
}
it emits:
demo.c: In function ‘test_arr’:
demo.c:7:3: warning: ‘putenv’ on a pointer to automatic variable ‘arr’ [POS34-C] [-Wanalyzer-putenv-of-auto-var]
7 | putenv (arr);
| ^~~~~~~~~~~~
‘test_arr’: event 1
|
| 7 | putenv (arr);
| | ^~~~~~~~~~~~
| | |
| | (1) ‘putenv’ on a pointer to automatic variable ‘arr’
|
demo.c:6:8: note: ‘arr’ declared on stack here
6 | char arr[] = "NAME=VALUE";
| ^~~
demo.c:7:3: note: perhaps use ‘setenv’ rather than ‘putenv’
7 | putenv (arr);
| ^~~~~~~~~~~~
gcc/analyzer/ChangeLog:
PR analyzer/105893
* analyzer.opt (Wanalyzer-putenv-of-auto-var): New.
* region-model-impl-calls.cc (class putenv_of_auto_var): New.
(region_model::impl_call_putenv): New.
* region-model.cc (region_model::on_call_pre): Handle putenv.
* region-model.h (region_model::impl_call_putenv): New decl.
gcc/ChangeLog:
PR analyzer/105893
* doc/invoke.texi: Add -Wanalyzer-putenv-of-auto-var.
gcc/testsuite/ChangeLog:
PR analyzer/105893
* gcc.dg/analyzer/putenv-1.c: New test.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
Diff:
---
gcc/analyzer/analyzer.opt | 4 ++
gcc/analyzer/region-model-impl-calls.cc | 117 +++++++++++++++++++++++++++++++
gcc/analyzer/region-model.cc | 6 ++
gcc/analyzer/region-model.h | 1 +
gcc/doc/invoke.texi | 14 ++++
gcc/testsuite/gcc.dg/analyzer/putenv-1.c | 109 ++++++++++++++++++++++++++++
6 files changed, 251 insertions(+)
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 5021376b6fb..808ff36ac54 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -126,6 +126,10 @@ Wanalyzer-null-dereference
Common Var(warn_analyzer_null_dereference) Init(1) Warning
Warn about code paths in which a NULL pointer is dereferenced.
+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.
+
Wanalyzer-shift-count-negative
Common Var(warn_analyzer_shift_count_negative) Init(1) Warning
Warn about code paths in which a shift with negative count is attempted.
diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc
index 8c38e9206fa..3f821ff07e1 100644
--- a/gcc/analyzer/region-model-impl-calls.cc
+++ b/gcc/analyzer/region-model-impl-calls.cc
@@ -549,6 +549,123 @@ region_model::impl_call_memset (const call_details &cd)
fill_region (sized_dest_reg, fill_value_u8);
}
+/* A subclass of pending_diagnostic for complaining about 'putenv'
+ called on an auto var. */
+
+class putenv_of_auto_var
+: public pending_diagnostic_subclass<putenv_of_auto_var>
+{
+public:
+ putenv_of_auto_var (tree fndecl, const region *reg)
+ : m_fndecl (fndecl), m_reg (reg),
+ m_var_decl (reg->get_base_region ()->maybe_get_decl ())
+ {
+ }
+
+ const char *get_kind () const final override
+ {
+ return "putenv_of_auto_var";
+ }
+
+ bool operator== (const putenv_of_auto_var &other) const
+ {
+ return (m_fndecl == other.m_fndecl
+ && m_reg == other.m_reg
+ && same_tree_p (m_var_decl, other.m_var_decl));
+ }
+
+ int get_controlling_option () const final override
+ {
+ return OPT_Wanalyzer_putenv_of_auto_var;
+ }
+
+ bool emit (rich_location *rich_loc) final override
+ {
+ auto_diagnostic_group d;
+ diagnostic_metadata m;
+
+ /* SEI CERT C Coding Standard: "POS34-C. Do not call putenv() with a
+ pointer to an automatic variable as the argument". */
+ diagnostic_metadata::precanned_rule
+ rule ("POS34-C", "https://wiki.sei.cmu.edu/confluence/x/6NYxBQ");
+ m.add_rule (rule);
+
+ bool warned;
+ if (m_var_decl)
+ warned = warning_meta (rich_loc, m, get_controlling_option (),
+ "%qE on a pointer to automatic variable %qE",
+ m_fndecl, m_var_decl);
+ else
+ warned = warning_meta (rich_loc, m, get_controlling_option (),
+ "%qE on a pointer to an on-stack buffer",
+ m_fndecl);
+ if (warned)
+ {
+ if (m_var_decl)
+ inform (DECL_SOURCE_LOCATION (m_var_decl),
+ "%qE declared on stack here", m_var_decl);
+ inform (rich_loc->get_loc (), "perhaps use %qs rather than %qE",
+ "setenv", m_fndecl);
+ }
+
+ return warned;
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) final override
+ {
+ if (m_var_decl)
+ return ev.formatted_print ("%qE on a pointer to automatic variable %qE",
+ m_fndecl, m_var_decl);
+ else
+ return ev.formatted_print ("%qE on a pointer to an on-stack buffer",
+ m_fndecl);
+ }
+
+ void mark_interesting_stuff (interesting_t *interest) final override
+ {
+ if (!m_var_decl)
+ interest->add_region_creation (m_reg->get_base_region ());
+ }
+
+private:
+ tree m_fndecl; // non-NULL
+ const region *m_reg; // non-NULL
+ tree m_var_decl; // could be NULL
+};
+
+/* Handle the on_call_pre part of "putenv".
+
+ In theory we could try to model the state of the environment variables
+ for the process; for now we merely complain about putenv of regions
+ on the stack. */
+
+void
+region_model::impl_call_putenv (const call_details &cd)
+{
+ tree fndecl = cd.get_fndecl_for_call ();
+ gcc_assert (fndecl);
+ region_model_context *ctxt = cd.get_ctxt ();
+ const svalue *ptr_sval = cd.get_arg_svalue (0);
+ const region *reg = deref_rvalue (ptr_sval, cd.get_arg_tree (0), ctxt);
+ m_store.mark_as_escaped (reg);
+ enum memory_space mem_space = reg->get_memory_space ();
+ switch (mem_space)
+ {
+ default:
+ gcc_unreachable ();
+ case MEMSPACE_UNKNOWN:
+ case MEMSPACE_CODE:
+ case MEMSPACE_GLOBALS:
+ case MEMSPACE_HEAP:
+ case MEMSPACE_READONLY_DATA:
+ break;
+ case MEMSPACE_STACK:
+ if (ctxt)
+ ctxt->warn (new putenv_of_auto_var (fndecl, reg));
+ break;
+ }
+}
+
/* Handle the on_call_pre part of "operator new". */
void
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index f7df2fca245..a140f4d5088 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1539,6 +1539,12 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt,
impl_call_memset (cd);
return false;
}
+ else if (is_named_call_p (callee_fndecl, "putenv", call, 1)
+ && POINTER_TYPE_P (cd.get_arg_type (0)))
+ {
+ impl_call_putenv (cd);
+ return false;
+ }
else if (is_named_call_p (callee_fndecl, "strchr", call, 2)
&& POINTER_TYPE_P (cd.get_arg_type (0)))
{
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 42f8abeb043..a9657e0200a 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -630,6 +630,7 @@ class region_model
void impl_call_malloc (const call_details &cd);
void impl_call_memcpy (const call_details &cd);
void impl_call_memset (const call_details &cd);
+ void impl_call_putenv (const call_details &cd);
void impl_call_realloc (const call_details &cd);
void impl_call_strchr (const call_details &cd);
void impl_call_strcpy (const call_details &cd);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 38ae900377e..e8cd60103e4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -459,6 +459,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-analyzer-null-dereference @gol
-Wno-analyzer-possible-null-argument @gol
-Wno-analyzer-possible-null-dereference @gol
+-Wno-analyzer-putenv-of-auto-var @gol
-Wno-analyzer-shift-count-negative @gol
-Wno-analyzer-shift-count-overflow @gol
-Wno-analyzer-stale-setjmp-buffer @gol
@@ -9761,6 +9762,7 @@ Enabling this option effectively enables the following warnings:
-Wanalyzer-null-dereference @gol
-Wanalyzer-possible-null-argument @gol
-Wanalyzer-possible-null-dereference @gol
+-Wanalyzer-putenv-of-auto-var @gol
-Wanalyzer-shift-count-negative @gol
-Wanalyzer-shift-count-overflow @gol
-Wanalyzer-stale-setjmp-buffer @gol
@@ -10017,6 +10019,18 @@ value known to be NULL is dereferenced.
See @uref{https://cwe.mitre.org/data/definitions/476.html, CWE-476: NULL Pointer Dereference}.
+@item -Wno-analyzer-putenv-of-auto-var
+@opindex Wanalyzer-putenv-of-auto-var
+@opindex Wno-analyzer-putenv-of-auto-var
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-possible-null-dereference} to disable it.
+
+This diagnostic warns for paths through the code in which a
+call to @code{putenv} is passed a pointer to an automatic variable
+or an on-stack buffer.
+
+See @uref{https://wiki.sei.cmu.edu/confluence/x/6NYxBQ, POS34-C. Do not call putenv() with a pointer to an automatic variable as the argument}.
+
@item -Wno-analyzer-shift-count-negative
@opindex Wanalyzer-shift-count-negative
@opindex Wno-analyzer-shift-count-negative
diff --git a/gcc/testsuite/gcc.dg/analyzer/putenv-1.c b/gcc/testsuite/gcc.dg/analyzer/putenv-1.c
new file mode 100644
index 00000000000..4c3f0ae2e74
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/putenv-1.c
@@ -0,0 +1,109 @@
+/* { dg-additional-options "-Wno-analyzer-null-argument" } */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+extern void populate (char *buf);
+
+void test_passthrough (char *s)
+{
+ putenv (s);
+}
+
+void test_str_lit (void)
+{
+ putenv ("NAME=value");
+}
+
+/* glibc allows strings without an equal sign. */
+
+void test_no_eq (void)
+{
+ putenv ("NAME");
+}
+
+void test_empty_string (void)
+{
+ putenv ("");
+}
+
+void test_NULL (void)
+{
+ putenv (NULL); /* possibly -Wanalyzer-null-argument */
+}
+
+void test_auto_buf_name_and_value (const char *name, const char *value)
+{
+ char buf[100]; /* { dg-message "'buf' declared on stack here" } */
+ snprintf (buf, sizeof (buf), "%s=%s", name, value);
+ putenv (buf); /* { dg-warning "'putenv' on a pointer to automatic variable 'buf' \\\[POS34-C\\\]" "warning" } */
+ /* { dg-message "perhaps use 'setenv' rather than 'putenv'" "setenv suggestion" { target *-*-* } .-1 } */
+}
+
+void test_auto_buf_value (const char *value)
+{
+ char buf[100]; /* { dg-message "'buf' declared on stack here" } */
+ snprintf (buf, sizeof (buf), "NAME=%s", value);
+ putenv (buf); /* { dg-warning "'putenv' on a pointer to automatic variable 'buf' \\\[POS34-C\\\]" } */
+}
+
+void test_static_buf (const char *value)
+{
+ static char buf[100];
+ snprintf (buf, sizeof (buf), "NAME=%s", value);
+ putenv (buf);
+}
+
+static char global_buf[1024];
+
+void test_global (const char *value)
+{
+ snprintf (global_buf, sizeof (global_buf), "NAME=%s", value);
+ putenv (global_buf);
+}
+
+void test_alloca (void)
+{
+ char *buf = __builtin_alloca (256); /* { dg-message "region created on stack here" } */
+ populate (buf);
+ putenv (buf); /* { dg-warning "'putenv' on a pointer to an on-stack buffer \\\[POS34-C\\\]" } */
+}
+
+void test_malloc_1 (void)
+{
+ char *buf = malloc (1024);
+ if (!buf)
+ return;
+ populate (buf);
+ putenv (buf);
+}
+
+void test_malloc_2 (void)
+{
+ const char *kvstr = "NAME=value";
+ size_t len = __builtin_strlen (kvstr);
+ char *buf = __builtin_malloc (len + 1);
+ if (!buf)
+ return;
+ __builtin_memcpy (buf, kvstr, len);
+ buf[len] = '\0';
+ putenv (buf); /* { dg-bogus "leak" } */
+}
+
+void test_arr (void)
+{
+ char arr[] = "NAME=VALUE"; /* { dg-message "'arr' declared on stack here" } */
+ putenv (arr); /* { dg-warning "'putenv' on a pointer to automatic variable 'arr' \\\[POS34-C\\\]" } */
+}
+
+static void __attribute__((noinline))
+__analyzer_test_inner (char *kvstr)
+{
+ putenv (kvstr); /* { dg-warning "'putenv' on a pointer to automatic variable 'arr_outer' \\\[POS34-C\\\]" } */
+}
+
+void test_outer (void)
+{
+ char arr_outer[] = "NAME=VALUE"; /* { dg-message "'arr_outer' declared on stack here" } */
+ __analyzer_test_inner (arr_outer);
+}
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-07-28 21:25 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-28 21:25 [gcc r13-1881] analyzer: new warning: -Wanalyzer-putenv-of-auto-var [PR105893] David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).