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]

Warning for printf (foo) (was: Re: Patch to add __builtin_printf)


On Tue, 19 Sep 2000, Kaveh R. Ghazi wrote:

>  > From: Zack Weinberg <zack@rabi.columbia.edu>
>  > 
>  > In light of recent security advisories, I'd like to see us do a
>  > transformation like this:
>  > 
>  >   char *foo;  printf (foo);  -> printf ("%s", foo);  [->fputs (foo, stdout)]
>  > 
>  > and issue a loud warning about the potential hole.  Note that the
>  > transformation only applies when there are no arguments after the variable.
> 
> I'm about to submit patches to achieve: printf("%s",foo)->fputs(foo,stdout)
> (Capturing stdout was the hairy part.)  So that much you can count on.
> 
> WRT printf(foo), warning about it is really a special case of
> -Wformat=2, so it shouldn't be hard.  (Though I still wish the author
> would have documented the new option...)

This patch adds a warning - not conditional on -Wformat=2 - for
non-literal formats if there are no arguments for the format
(e.g. printf (foo)).  It also adds documentation and testcases, and
stops -Wall from overriding -Wformat=2.

C compiler bootstrapped and C testsuite run with no regressions on
i686-pc-linux-gnu.  (libio doesn't build for me with the current CVS tree
with glibc 2.1.94 installed.)  OK to commit?

gcc/ChangeLog:
2000-09-20  Joseph S. Myers  <jsm28@cam.ac.uk>

	* c-common.c (check_format_info): If the format is not a string
	literal and there are no arguments to the format, give a warning
	without needing -Wformat=2.
	* c-decl.c (c_decode_option): If warn_format is nonzero, don't
	change it in response to -Wall, so -Wall does not override
	-Wformat=2.
	* invoke.texi: Document -Wformat=2.

gcc/testsuite/ChangeLog:
2000-09-20  Joseph S. Myers  <jsm28@cam.ac.uk>

	* gcc.dg/format-nonlit-1.c, gcc.dg/format-nonlit-2.c: New tests.

--- c-common.c.orig	Tue Sep 19 19:01:11 2000
+++ c-common.c	Tue Sep 19 22:40:38 2000
@@ -2024,8 +2024,22 @@
       /* Functions taking a va_list normally pass a non-literal format
 	 string.  These functions typically are declared with
 	 first_arg_num == 0, so avoid warning in those cases.  */
-      if (info->first_arg_num != 0 && warn_format > 1)
-	status_warning (status, "format not a string literal, argument types not checked");
+      if (info->first_arg_num != 0)
+	{
+	  /* If there are no arguments for the format at all, we may have
+	     printf (foo) which is likely to be a security hole.  */
+	  while (arg_num + 1 < info->first_arg_num)
+	    {
+	      if (params == 0)
+		break;
+	      params = TREE_CHAIN (params);
+	      ++arg_num;
+	    }
+	  if (info->format_type != strftime_format_type && params == 0)
+	    status_warning (status, "format not a string literal and no format arguments");
+	  else if (warn_format > 1)
+	    status_warning (status, "format not a string literal, argument types not checked");
+	}
       return;
     }
   format_tree = TREE_OPERAND (format_tree, 0);
@@ -2036,8 +2050,22 @@
       /* Functions taking a va_list normally pass a non-literal format
 	 string.  These functions typically are declared with
 	 first_arg_num == 0, so avoid warning in those cases.  */
-      if (info->first_arg_num != 0 && warn_format > 1)
-	status_warning (status, "format not a string literal, argument types not checked");
+      if (info->first_arg_num != 0)
+	{
+	  /* If there are no arguments for the format at all, we may have
+	     printf (foo) which is likely to be a security hole.  */
+	  while (arg_num + 1 < info->first_arg_num)
+	    {
+	      if (params == 0)
+		break;
+	      params = TREE_CHAIN (params);
+	      ++arg_num;
+	    }
+	  if (info->format_type != strftime_format_type && params == 0)
+	    status_warning (status, "format not a string literal and no format arguments");
+	  else if (warn_format > 1)
+	    status_warning (status, "format not a string literal, argument types not checked");
+	}
       return;
     }
   format_chars = TREE_STRING_POINTER (format_tree);
--- c-decl.c.orig	Tue Sep 19 08:34:34 2000
+++ c-decl.c	Tue Sep 19 22:55:23 2000
@@ -811,7 +811,8 @@ c_decode_option (argc, argv)
       warn_return_type = 1;
       set_Wunused (1);
       warn_switch = 1;
-      warn_format = 1;
+      if (warn_format == 0)
+	warn_format = 1;
       warn_char_subscripts = 1;
       warn_parentheses = 1;
       warn_missing_braces = 1;
--- invoke.texi.orig	Sat Sep 16 20:33:33 2000
+++ invoke.texi	Tue Sep 19 23:06:05 2000
@@ -1494,7 +1494,15 @@
 @item -Wformat
 Check calls to @code{printf} and @code{scanf}, etc., to make sure that
 the arguments supplied have types appropriate to the format string
-specified.
+specified.  If @samp{-Wformat=2} is specified, also warn if the format
+string is not a string literal and so cannot be checked, unless the
+format function takes its format arguments as a @code{va_list}.  (Note
+that @samp{-Wformat=2} is not included in @samp{-Wall}.  However, the
+special case where the format string is not a string literal and there
+are no format arguments, as in @code{printf (foo);}, is warned about
+with plain @samp{-Wformat} or @samp{-Wall}, since it may be a security
+hole if the format string came from untrusted input and contains
+@code{%n}.)
 
 @item -Wimplicit-int
 Warn when a declaration does not specify a type.
--- /dev/null	Fri Sep 11 11:31:59 1998
+++ format-nonlit-1.c	Tue Sep 19 22:45:58 2000
@@ -0,0 +1,13 @@
+/* Test for warnings for non-string-literal formats.  Test with -Wformat=2.  */
+/* Origin: Joseph Myers <jsm28@cam.ac.uk> */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99 -Wformat=2" } */
+
+extern int printf (const char *, ...);
+
+void
+foo (char *s, int i)
+{
+  printf ((const char *)i, i); /* { dg-warning "argument types" "non-literal" } */
+  printf (s, i); /* { dg-warning "argument types" "non-literal" } */
+}
--- /dev/null	Fri Sep 11 11:31:59 1998
+++ format-nonlit-2.c	Tue Sep 19 22:47:39 2000
@@ -0,0 +1,14 @@
+/* Test for warnings for non-string-literal formats.  Test for security
+   warning when format has no arguments.
+*/
+/* Origin: Joseph Myers <jsm28@cam.ac.uk> */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99 -Wformat" } */
+
+extern int printf (const char *, ...);
+
+void
+foo (char *s)
+{
+  printf (s); /* { dg-warning "no format arguments" "security warning" } */
+}

-- 
Joseph S. Myers
jsm28@cam.ac.uk


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