Overflows due to unexpected casts

[mixter () NEWYORKOFFICE COM (Mixter)]

I have lately examined some of my own source, and the source of mpg123,
because I had experienced some strange segv's. I found that the reason of
some segmentation faults were buffer overflows caused by bad conversions.

Almost all bounds checking functions like like memcpy, strncpy, snprintf,
etc. use integers which are, of course, unsigned, for the buffer size
argument. However, programs commonly use to pass signed integers or
signed long integers to those functions. In the rare, but possible condition
that such an integer is <= 0, the conversion to an unsigned int (which is done
implicity by passing it as argument to the bounds checking function) will
result in unexpected behavior... the LSB that is used to determine signedness
is a part of the numeric value in unsigned variables, which results in...

(signed int) -1 == (unsigned int) UINT_MAX
(printf ("%u\n", -1); will show that UINT_MAX is usually 4294967295)

I found that this is the cause of a segfault in mpg123 ver. 0.59q,
which happens when invalid layer3 data is read from an mpeg file...
do_layer3()/layer3.c fetches header information, including a length value,
from the mpeg3 stream, and calls set_pointer()/common.c with this value
without further checking it, which looks like this:

void set_pointer(long backstep)
{
  wordpointer = bsbuf + ssize - backstep;
  if (backstep)
    memcpy(wordpointer,bsbufold+fsizeold-backstep,backstep);
  bitindex = 0;
}

Note that the function performs a check, which is however insufficient..
"if (backstep) ..." will only prevent the integer from being exactly 0, not
from having a negative value. Therefore, if receiving -1 as buffer length,
memcpy can be instructed to copy up to 4294967295 bytes from src to dest.
This is obviously a bad thing, and I think that many programs could suffer
from this condition, if they do not check for values smaller than zero,
before passing them to functions that treat them as unsigned integers.

Please note that the impact in mpg123 is minimal, as it probably can only
be crashed. It is just used to illustrate the problem that might be present
in other applications.

A thing which I am not certain about is, could such overflows be used to
execute arbitrary code? E.g., will they always segfault prior to the point
of copying the huge strings to the destination, or do they first try to
return, leaving the possibility of executing code on the stack? Anyways,
I think this kind of conversion needs to be done with care in server
applications which determine the input size in any way by obtaining it
from the data stream (e.g. DNS protocol), because there could be at least
the possibility to remotely crash server applications with this bug.

Mixter
________________________
mixter () newyorkoffice com
http://1337.tsx.org
mkdir -p `perl -e 'printf "a/" x 1000'`