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]

Re: Patch to add __builtin_printf


On Tue, 19 Sep 2000 16:21:08 -0400 (EDT), "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.

Hm... Appears to me you're working too hard.  You know you are using GCC's
preprocessor, therefore you can inject

#undef printf
#define printf(args...) fprintf(stdout, args...)

at the end of stdio.h.  It's probably easiest to do this with fix-header,
which knows how to insert things inside a multiple-include wrapper.  (But
note fix-header is not run on all targets.)

Then you have the expansion of stdout right where you need it in the
builtins.c logic.  [It may be worthwhile to turn fprintf(stdout, ...) back
into printf(...) - at least for -Os - but that's trivial.]

This doesn't help you capture putc, of course.  You could also wait for (or
help finish) the integrated preprocessor, and then dig through its symbol
table from the front end.  You'd also need a way of asking the parser 'scan
this token list and give me the tree for it, please' - C++ can almost do
this now, see spew.c.

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

Oh, is _that_ where the 'format not a string literal' warning went?  No
wonder...

I like Joseph's patch that turns it back on for printf (foo) at -Wall.

>Anyway, whether to automatically do the transformation you suggest on
>printf(foo) is debatable IMHO given that the original is a legal (but
>admittedly dangerous) use, and the transformation by nature isn't
>equivalent.  (E.g. "%%" gets printf-processed in the original style.)

Gah, I forgot about %%.  You're right, the transform is not safe.

One other thing - some people like to write

static const char nothing_happens[] = "Nothing happens.\n"

...

printf(nothing_happens);

Be nice if we went and fished the text out of the VAR_DECL when we could,
and checked arguments as normal.  [A bare "extern const char *nothing_happens"
should still get warned about - you don't know where the initializer will
come from, it could be unchecked user input.]

zw

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