This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC] Warning for potentially unbound writes to function parameters


In some real-world code, I noticed a curious pattern: using the unsafe string functions on function parameter arguments. This leads to gets()-style unsafe APIs.

I've looked at how to implement a warning for this, and came up with the attached patch. Do you think this makes sense?

     1	#include <string.h>
     2	
     3	const char *data (void);
     4	
     5	void test (char *target)
     6	{
     7	  strcpy(target, data ());
     8	}
     9	
    10	
    11	void test_2 (char *target)
    12	{
    13	  char *p = target;
    14	  strcpy(p, data ());
    15	}
    16	

/tmp/t.c: In function ‘test’:
/tmp/t.c:7:9: warning: potentially unbound write to function parameter ‘target’ [-Wunbound-parameter-write]
strcpy(target, data ());
^
/tmp/t.c: In function ‘test_2’:
/tmp/t.c:14:9: warning: potentially unbound write to function parameter ‘target’ [-Wunbound-parameter-write]
strcpy(p, data ());
^


Obviously, the warning and its name need adjusting, and more functions need to be covered. But I want to check first if you think the warning makes sense at all, and if I've found the right place to implement it (this approach seems to require optimization, alas).

--
Florian Weimer / Red Hat Product Security Team
commit 324c7189c9cf871584da988f12d1a686df0d6e0c
Author: Florian Weimer <fweimer@redhat.com>
Date:   Fri Aug 17 18:19:13 2012 +0200

    Implement -Wunbound-parameter-write (proof of concept)

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 4b177c4..dc90484 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -3274,6 +3274,14 @@ expand_builtin_strcpy (tree exp, rtx target)
    {
      tree dest = CALL_EXPR_ARG (exp, 0);
      tree src = CALL_EXPR_ARG (exp, 1);
+     if (TREE_CODE (dest) == SSA_NAME)
+       {
+	 tree dest_var = SSA_NAME_VAR (dest);
+	 if (TREE_CODE (dest_var) == PARM_DECL)
+	   warning_at (EXPR_LOCATION (exp), OPT_Wunbound_parameter_write,
+		       "potentially unbound write to function parameter %qD",
+		       dest_var);
+       }
      return expand_builtin_strcpy_args (dest, src, target);
    }
    return NULL_RTX;
diff --git a/gcc/common.opt b/gcc/common.opt
index deb89e3..fe892b7 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -562,6 +562,10 @@ Wlarger-than=
 Common RejectNegative Joined UInteger Warning
 -Wlarger-than=<number>	Warn if an object is larger than <number> bytes
 
+Wunbound-parameter-write
+Common Var(warn_unbound_parameter_write) Warning
+Warn if a function without array bounds checking writes to a pointer passed as an parameter
+
 Wunsafe-loop-optimizations
 Common Var(warn_unsafe_loop_optimizations) Warning
 Warn if the loop cannot be optimized due to nontrivial assumptions.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]