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 <fwei...@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.

Reply via email to