Security patch for printf - please no! (Was: Re: Patch to add__builtin_printf)

[Kamil Iskra <kamil at wins dot uva dot nl>]

On Tue, 19 Sep 2000, Zack Weinberg wrote:

> 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 am sorry, but I think that this idea, and the whole thread following it,
patches included, is nonsensical.

Implementing this idea will result in:

1. HUGE amounts of false positives for most code.

2. false sense of security, because in fact only one category of format
   string problems will be patched.

You think that "printf(var);" is dangerous, so you suggest to change it to
"printf("%s", var);". Sure, that will secure it, although I have some
reservations if an optimising compiler should perform such
anti-optimisations.

However, you fail to patch other obvious examples of format string
problems. What's the difference between the two lines:

printf(var);
printf(var, "abc");

The first one is vulnerable to var such as "bla %s bla", the second one to
only slightly more complicated "bla %s %s bla". You can fix the first one,
but what about the second one? You are not going to suggest that GCC
suddenly starts warning whenever the format string is not a constant?

Basically, you can't fix such format string problems this way. Given the
large noise raitio which must result from such a patch, and the minimal
improvements it results in, I strongly suggest that the GCC maintainers
reject it. It just isn't worth the trouble it causes.

-- 
/ Kamil Iskra  kamil@wins.uva.nl                                          \
| Section Computational Science, Faculty FNWI, Universiteit van Amsterdam |
| tel. +31 20 525 75 35  fax. +31 20 525 74 90                            |
\ Kruislaan 403  room F.202  1098 SJ Amsterdam (NL)                       /