Re: Debug mode output broken

[Jonathan Wakely <cow at compsoc dot man dot ac dot uk>]

On Fri, Aug 06, 2004 at 01:16:50PM +0200, Paolo Carlini wrote:

> Hi again Jonathan,
> 
> and, first, thanks for your work.
> 
> >Shall I commit this to 3.4 and mainline?
> > 
> >
> No, sorry. It appears to paper over the real bug. Indeed, according to 
> the C standard
> sprintf automatically writes a null char at the end of the characters 
> written.

But the problem is that sprintf() writes far more than __n chars to
__buf (although not more than __bufsize in this particular case).
So the null is there, but in the wrong place, so when __buf is read from
more chars than intended are read (this is what caused the error message
to be displayed three times - each time a word was supposed to be
printed the entire string was printed)

> It looks like for some reason the capacity of __buf is exceeded and the 
> null char is
> written beyond the end of the buffer! A very dangerous situation, by the 
> way.

I don't think that is happening here, although it's certainly possible
for it to happen if the _S_debug_messages string is too long.

What's supposed to happen is that __n chars are copied to __buf,
followed by '\0'. Then __buf is passed to fprintf().

When snprintf() is used the '\0' is put in at __buf[__n-1], and fprintf()
only reads [0,__n) from __buf.  This is correct.

When snprintf() is _not_ available, the '\0' is not at __buf[__n-1], but
at some later position in __buf; I think it's at __buf[strlen(__s)].

So if strlen(__s) >= __bufsize then BOOM, otherwise we just read
[0,strlen(__s)) from __buf, resulting in duplicated words on stderr.

My first patch (to insert '\0' at __buf[__n-1]) gives the right answer,
but doesn't prevent buffer overflow if strlen(__s) >= __bufsize.

My suggestion to add an assertion to check strlen(__s) should be enough
to prevent buffer overflows (in this code path, at least).

jon

-- 
"A woman drove me to drink, I never had the courtesy to thank her."
	- W.C. Fields