[Pkg-scicomp-devel] Bug#441478: libglpk0: security flaw buffer overflow in glplib05.c xvprintf

[Peter T. Breuer]

Package: libglpk0
Version: 4.20-1
Severity: minor


Looking through the code for a way to shut off the annoying messaged
from lpx_adv_basis in the version of libglpk0 that is current for my
distribution (a new version has been made available to unstable, but
that's not me ...), I noticed the following in xvprintf of
src/glplib05.c:

  static void
  xvprintf (const char *fmt, va_list arg)
  {
      char    buf[4000 + 1];
      vsprintf (buf, fmt, arg);
      xassert (strlen (buf) < sizeof (buf));          /* here! */
      xputs (buf);
      return;
  }

The assertion checks the length of the string in the current buffer
AFTER having written it there. Too late, and ineffective anyway.

1) The buffer overflow has already occurred, if it has occurred at all,
   so the check is notionally too late. One wants to check before
   doing the vsprintf into the buffer, if anywhere.
   
   Yes, it is likely that a buffer overflow seeks to alter the return
   address on the stack, so a check in the same routine is not too late
   for deetcting that, but one can perfectly easily write a string with
   a zero half-way along (by writing a low integer, for example) that is
   going to stop the strlen calculation within bounds, and a buffer
   overflow attempt WILL write zeros.

2) In any case checking strlen(buf) will overrun the buffer in the
   event the test fails, likely resulting in a violation of another
   sort as it eventually runs into un-mapped memory areas. Only
   luck stops it segfaulting.

3) The correct way to do this is to use vsnprintf. One wants

      vsnprintf(buf, sizeof(buf), fmt, arg);

   The sizeof(buf) is correct. Not sizeof(buf)-1 as the count by
   vsnprintf includes a trailing zero. And in any case one can
   check the number of bytes returned:

      int n = vsnprintf(buf, sizeof(buf), fmt, arg);
      xassert(n <= sizeof(buf));

if one really wanted to do a check AFTER the event :), useless though
that is.

Best

Peter


-- System Information:
Debian Release: lenny/sid
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: i386 (i686)

Kernel: Linux 2.6.15.3 (PREEMPT)
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968) (ignored: LC_ALL set to C)
Shell: /bin/sh linked to /bin/bash

Versions of packages libglpk0 depends on:
ii  libc6                     2.6.1-1+b1     GNU C Library: Shared libraries
ii  libgmp3c2                 2:4.2.1+dfsg-5 Multiprecision arithmetic library

libglpk0 recommends no packages.

-- no debconf information