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>
This commit is contained in:
David Malcolm
parent
9c60338061
commit
872693eebb
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)))
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
Loading…
Reference in New Issue