Debug mode output broken

[Jonathan Wakely]

On Fri, Aug 06, 2004 at 12:12:50PM +0100, Jonathan Wakely wrote:

> On Fri, Aug 06, 2004 at 11:57:04AM +0100, Jonathan Wakely wrote:
> 
> > The reason it works on linux and not elsewhere seems to be this:
> > 
> >   template<typename _Tp>
> >     void
> >     _Error_formatter::_M_format_word(char* __buf,
> >                      int __n __attribute__((__unused__)),
> >                      const char* __fmt, _Tp __s) const
> >     {
> > #ifdef _GLIBCXX_USE_C99
> >       std::snprintf(__buf, __n, __fmt, __s);
> > #else
> >       std::sprintf(__buf, __fmt, __s);
> > #endif
> >     }
> > 
> > If sprintf is used the whole string is copied to the buffer, not just
> > the first word (or more correctly, the first __n chars).
> 
> Is it possible to inject strings into the error messages used by the
> debug mode?  If so it would be possible to cause a buffer overflow here.
> An inadvertent change to a message that makes it longer than the
> 128-byte buffer would also cause an overflow. Badness.
> 
> e.g. __gnu_debug::_S_debug_messages[10] :
> 
>     "elements in iterator range [%1.name;, %2.name;)"
>     " are not sorted according to the predicate %3;"
> 
> How long can the three params here get when expanded?
> 54 chars or more would cause problems IIUC.

Except _M_format_word() isn't called on the expanded string, so that's
not a problem. The unexpanded string still needs to be short enough to
fit in the buffer though.

Maybe this should be added to _M_print_string(), before the while loop:

#ifndef _GLIBCXX_USE_C99
    // _M_format_word() uses sprintf() and __string might overflow __buf
    assert(std::strlen(__string) < __bufsize);
#endif

I don't think an extra strlen() is going to be a performance problem
seeing as we're processing a failed debug-mode assertion at this point.


AFAICT FreeBSD has had snprintf() since version 1.0 (see [1])
so the test _could_ be changed to:

#if defined( _GLIBCXX_USE_C99 ) || defined( __FreeBSD__ )

and possibly others too, although I realise this breaks the conventions
used in libstdc++, and breaks the KISS principle.

[1] http://www.freebsd.org/cgi/man.cgi?query=snprintf&apropos=0&sektion=0&manpath=FreeBSD+1.0-RELEASE&format=html

jon

-- 
"Subvert the dominant paradigm!"
	- The I.O.D.