This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Debug mode output broken


On Fri, Aug 06, 2004 at 02:14:30PM +0200, Paolo Carlini wrote:

> Jonathan Wakely wrote:
> 
> >What's supposed to happen is that __n chars are copied to __buf,
> >followed by '\0'.
> 
> *at most* __n chars are copied to __buf: that's the semantics of snprintf.
> 
> >My first patch (to insert '\0' at __buf[__n-1]) gives the right answer,
> >but doesn't prevent buffer overflow if strlen(__s) >= __bufsize.
> > 
> >
> It's not only that doesn't prevent the overflow.
> 
> You have not explained *why* you have to add by hand that '\0'. This is 
> never
> supposed to happen with functions that always add a '\0' automatically.

First of all, please note that __n is not the buffer size here, it's the
length of the word to be copied. snprintf() is not used to prevent __buf
from being overflowed, it's used to prevent the whole of __s being copied.

sprintf() *does* add a '\0', but in the wrong place. These two
lines are simply *not* equivalent when __n < strlen(__s):

#ifdef _GLIBCXX_USE_C99
      std::snprintf(__buf, __n, __fmt, __s);
#else
      std::sprintf(__buf, __fmt, __s);
#endif

When that code runs __n is ALWAYS < strlen(__s) (because __n is found
by counting the printable chars in __s) and so the non-C99 version is
ALWAYS going to do something different to the C99 version.

My patch is intended to put the '\0' at the same place as the C99 version
does. It's still not the same, because it copies more than __n chars,
and then truncates at __n. As long as copying the extra chars doesn't
result in a buffer overflow my patch makes the non-C99 version
effectively equivalent to the C99 one, if not actually identical.

> The real fix involves understanding *why* there is junk between 
> __buf[__n - 1]
> and the position in the allocated buffer where sprintf actually places 
> the '\0'.

It's not junk - it's the rest of __s, but it's unwanted /at/this/time/.

_M_format_word() is supposed to copy the first __n chars from __s to
__buf, followed by '\0'. This is done so that a word at a time can be
copied from __s and printed. If more than one word is copied into __buf,
and not terminated, then more than one word will be printed. This is a
bug.

> You have not explained that.

I hope I have now - I'm trying to do two things at once and apologise if
I've been unclear.

jon

-- 
"They didn't know what the symbols and paradoxes meant. Instead of following
 the finger that points to the moon, they sat down and worshipped the finger
 itself. Instead of following the map, they thought it was the territory and
 tried to live in it. Instead of reading the menu they tried to eat it. Dig?"
	- Miss Portinari


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