On Tue, 2008-12-30 at 10:41 -0600, Steve Langasek wrote:
> On Tue, Dec 30, 2008 at 10:06:41AM +0100, Arthur de Jong wrote:
> > On Sun, 2008-12-28 at 12:02 -0600, Steve Langasek wrote:
> > > I don't know whether these are also a problem in practice - but if so,
> > > using sprintf(buf + strlen(buf) [...]) is definitely wrong.
>
> > I don't know if any of my code uses such a construct but why is that
> > wrong as long as [...] doesn't contain buf?
>
> That's not the context of this discussion; we were talking about buggy code
> that *did* use buf as one of the args to the format string.
Ok, I misunderstood. Thanks.
In that case there can be a great number of situations in which the
buffer may be filled with it's own content (eg. pass the same array as
two arguments to a function and use the above construct in that
function, assignments to temporary variables, etc, etc). Very hard to
check for.
Perhaps this would be a good test for tools such as split, rats or
flawfinder (none of them currently warn about this problem and neither
do any gcc flags that I commonly use). Those kind of tools already to
quite some analysis of source code so perhaps it isn't too difficult to
implement it there.
I've just performed a test with the following code on my system (sid,
hardening-wrapper not installed, compiled with gcc without any extra
flags):
char buf[20];
strcpy(buf,"FOO");
snprintf(buf,sizeof(buf),"%s%s",buf,"BAR");
printf("%s\n",buf);
strcpy(buf,"BAR");
snprintf(buf,sizeof(buf),"%s%s","FOO",buf);
printf("%s\n",buf);
which returned
BAR
FOOFOO
so in any case the behaviour is not as could be naively expected
(FOOBAR).
--
-- arthur - adejong@debian.org - http://people.debian.org/~adejong --
Attachment:
signature.asc
Description: This is a digitally signed message part