new class of printf issue: int overflow

[Felix von Leitner <felix-fulldisclosure () fefe de>]

This is about two issues.  First: abs within vasprintf.

I just read some gnupg source code and stumbled upon their
vasprintf implementation.  Basically they make one pass over the format
string to find out how much memory to malloc, and then they call sprintf
on the malloced buffer.

Here is an excerpt:

              if (*p == '*')
                {
                  ++p;
                  total_width += abs (va_arg (ap, int));
                }

I noticed this code because it calls abs on an int.  A little known fact
about abs is that it can return negative values.  If the int is
0x80000000, for example, abs() will also return 0x80000000.  So, I
thought, mhh, if someone can control this value but not the format
string, he can cause total_width to be very small but then write lots of
stuff to it.



Second issue: int overflow in *printf:

But that got me thinking.  *printf return an int, and it's supposed to
be the number of chars written.  So a typical idiom is

  size_t memory_needed=snprintf(NULL,0,format_string,...);
  char* ptr=malloc(memory_needed+1);
  sprintf(ptr,format_string,...);

What if sprintf returns a negative value?  printf can return -1 if
write() failed, but sprintf can not traditionally return -1.  The single
unix specification says this about sprintf:

     If the value of n is zero on a call to snprintf(), nothing shall be
     written, the number of bytes that would have been written had n been
     sufficiently large excluding the terminating null shall be returned, and
     s may be a null pointer.

So my guess is that nobody expects sprintf to return a negative value.
Out of curiosity, I wrote this test program:

  $ cat > t.c
  #include <stdio.h>

  int main() {
    printf("%d\n",snprintf(0,0,"%*d %*d",0x40000000,1,0x40000000,1));
  }
  $ gcc -o t t.c
  $ ./t
  -2147483647
  ./t  17.02s user 0.03s system 99% cpu 17.161 total
  $

the second line comes from my zsh, and as you can see running this
program took 17 seconds.  top shows that the process used 1 gig of
memory while it ran. :-)

This scenario is pretty far fetched, obviously.  In the above code
snippet, returning a negative number doesn't hurt, because malloc will
interpret it as unsigned really big number.  But hey, there could be
other scenarios.

The question is: do we want to do something about it?  What should
printf do if it detects an int overflow?  Return -1?  Is there a good
solution to this?  Solaris apparently returns -1.

Felix

PS: Does anyone understand why sprintf does not count the \0 at the end?
I think that's pretty brain-dead.

_______________________________________________
Full-Disclosure - We believe in it.
Charter: http://lists.grok.org.uk/full-disclosure-charter.html
Hosted and sponsored by Secunia - http://secunia.com/