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

[mark at codesourcery dot com]

Nonsensical is a strong term.

However, I agree -- the compiler is no substitute for
a security audit or a sophisticated security tool, and
we should not try to make it one.  The false positive
rate will be far too high to be useful.  

It would be useful to have a separate tool that did this
kind of thing, based on the same parsers used in GCC.
That's why we're trying (gradually) to separate the
front-ends from the back-ends; they would then be extremely
useful in other projects.

Our own Benjamin Chelf has done some great work (with 
Dawson Engler and other folks at Stanford) where they've
found lots of bugs in the Linux kernel by adding 
domain-specific logic to GCC.  Similar work could be
done for common security issues -- but this stuff should
not go into the compiler itself.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com


----- Original Message -----
Sent   : 9/21/00 1:51 PM
From   : Kamil Iskra <kamil@wins.uva.nl>
To     : Zack Weinberg <zack@rabi.columbia.edu>
CC     : Kaveh Ghazi <ghazi@caip.rutgers.edu>, gcc-patches@gcc.gnu.org
Subject: Security patch for printf - please no! (Was: Re: Patch to add __builtin_printf)

> 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)                       /


____________________________________________________

Get MORE than just FREE e-mail. Get a private Web office.
Organize your team with FREE communication and
collaboration tools from HotOffice(R) at  www.hotoffice.com.