Full Disclosure mailing list archives

Re: new class of printf issue: int overflow


From: Pierre Habouzit <madcoder () debian org>
Date: Thu, 11 Jan 2007 16:15:02 +0100

On Thu, Jan 11, 2007 at 03:45:01PM +0100, Pierre Habouzit wrote:
On Thu, Jan 11, 2007 at 03:18:21PM +0100, Felix von Leitner wrote:
Thus spake Pierre Habouzit (madcoder () debian org):
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,...);
  that's not the sole braindead idiom that generate errors. In my
software I use an xmalloc that returns NULL if its argument is <= 0,

That does not help.  The negative value is just an example, there could
also be a complete 32-bit overflow leading to snprintf returning 23
although it was going to write more than 4 GB.

  well, the bug would then lie in snprintf, and the sole remaining one
would be that snprintf could return MAX_INT, then MAX_INT+1 is (on every
usual machine) MIN_INT, and then my xmalloc would refuse to do anything
stupid :)

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.
  like said for your aprintf case, IMHO, MIN_INT for a '*' width
specifier has to be taken as an erroneous value. At least, it really
feels sensible.

The example had two %.d statements, neither of the * values was MIN_INT.

  Fair enough. I overlooked the fact that they even do not check for
size overflows. I always think unsigned integers are evil because
testing for an addition overflow is painful. in unsigned arithmetics,
a + b overflows iff MIN(a, b) < a +b. if you use signed operands, a + b
overflows iff a + b < 0, which is way more easy to test.

  err obviously I presupposed that we spoke about already valid sizes,
meaning that a and b would be both positive :) This was not meant as a
general affirmation that is obviously wrong.

  Sadly, most of the usual C API that take sizes use size_t and not
ssize_t, obviously because once upon a time, size_t have been only 16
bits big and that losing a bit did really matter...

  Since 32 (and now 64 bits) integers do not really have the same
limitations, I'm too used to use signed integers (especially to address
those issues in a trivial way), and forgot about that problem. So IOW,
you're right :)
-- 
·O·  Pierre Habouzit
··O                                                madcoder () debian org
OOO                                                http://www.madism.org

Attachment: _bin
Description:

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

Current thread: