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>

6 files changed, 251 insertions, 0 deletions

diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 5021376..808ff36 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 8c38e92..3f821ff 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 f7df2fc..a140f4d 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 42f8abe..a9657e0 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 38ae900..e8cd601 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 0000000..4c3f0ae
--- /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);
+}