diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index 5021376b6fb6c148b6632939b6cf4b3d9a2770e9..808ff36ac54012815759d5d254f5de99befcf451 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 8c38e9206fa6e43be8925bedcec7cee434b4b533..3f821ff07e1f15401dda6b4748a8e59b07dc44c6 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 f7df2fca2452024c622b029ff2d6f0bf71cf1c81..a140f4d5088ae1e5ede5216125545426d4e758f0 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 42f8abeb043a5f5f559502f7d3eed11a10f191c9..a9657e0200ad368bcfc91720259eedae0d2d6684 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 38ae900377e2b161c57c6d53c6d85e38464271a8..e8cd60103e4f537d06f5e1018c0feb7f14164e56 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 0000000000000000000000000000000000000000..4c3f0ae2e740609ca938b0d7bd5efbad4c99d0d2 --- /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); +}