[uClibc] Noted problem with vsnprintf() in 0.9.20

[Manuel Novoa III]

Hello,

On Wed, Jul 23, 2003 at 06:55:23PM -0700, Dean Matsen wrote:
> Hope this is not a repeat posting, but I didn't see
> any search engine to search the archives...

Coincidentally, I just posted something on this to the uClinux list.

http://mailman.uclinux.org/pipermail/uclinux-dev/2003-July/019647.html

> In version 0.9.20, I noticed that my telnetd (from
> netkit-telnet-0.17, using a recent buildroot) server was
> locking up. 
> 
> The problem, it turns out (I THINK), is that telnetd
> puts some non-printable characters in the format strings. 

Yes.  See below.

> This causes vsnprintf() to return -1, and ultimately
> causes telnetd to infinite loop (said loop is obviously
> intended to wait until the buffer is flushed, not until
> the vsnprintf call works...).

Sigh.. No one ever checks return values on *printf()...

> For example, telnetd declares a format string like this:
> 
>  static unsigned char doopt[] = { IAC, DO, '%', 'c', 0 };
> 
> and then tries to do
> 
>  char c;
> 
>  vsnprintf ( buffer, maxsize, (char *)doopt, c );
> 
> (where IAC = 255 and DO = 253, the offending control
> characters)

Regarding format strings for the *printf functions,  ANSI/ISO C99 standard
(and C89 too) states 
   The format shall be a multibyte character sequence, beginning and ending
   in its initial shift state.
This is also true of the format strings for the *scanf and strftime
functions.

The codeset for the C locale in uClibc is ASCII.  Any char with a
value outside the range of 0-0x7f is treated as an invalid multibyte
sequence since there is no associated wchar_t mapping.  This is actually
less restrictive than it could be, as ASCII is a superset of the portable
C codeset.

> For now, I modified the code to use a "%c" and pass the
> control character as a parameter, ie:
> 
>  vsnprintf ( buffer, maxsize, "%c%c%c", IAC, DO, c );

That's fine.  According to the standard, %c and %s simply treat their
data as bytes.  Multibyte considerations don't come into play.

Of course, using vsnprintf to assign 4 bytes in a buffer is pretty
inefficient...

> (which makes more sense anyway, but hey, this is THE
> telnetd code, right? so who am I to propose that someone
> fix it?).

Why not?  It is broken.  The restrictions on format strings have been
around since C89 at least.  Even if uClibc can be configured to not
check in the C locale, wouldn't you rather fix it in the app now than
have the app mysteriously fail when run in some UTF-8 codeset locale
for instance?

> Anyway, depending on the goals of uClibc, I recommend
> that vsnprintf NOT fail in this case, since it pertains
> to a very prominent daemon program!

As I mentioned in my post to the uClinux list, I will probably make
checking the format string when in the C locale a uClibc configuration
option, since there are so many broken programs out there.  However,
format strings have to be checked in locales using other codesets.
So this is just postponing the inevitable fixing of the app itself.

I do not intend my code in uClibc to be as 'tolerant' of standards
violations as glibc is; at least not be default.  Bad format strings
are (potential) application _bugs_ and need to be found and fixed.
The same could be said for allowing signed chars with negative values
to be accepted by the ctype functions.  glibc allows this to support
'old broken programs'.  In the next uClibc release, such support will
be configurable, since by allowing this you can never be sure if
the value passed was supposed to be ((unsigned char)(-1)) or -1,
which is EOF in most libc implementations.  (And yes, I've even
considered making the value of EOF configurable... just to see what
breaks.)

Manuel