full-disclosure - Re: new class of printf issue: int overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070111144501.GB20797@mad.intersec.eu>
Date: Thu, 11 Jan 2007 15:45:01 +0100
From: Pierre Habouzit <madcoder@...ian.org>
To: full-disclosure@...ts.grok.org.uk
Subject:  Re: new class of printf issue: int overflow

On Thu, Jan 11, 2007 at 03:18:21PM +0100, Felix von Leitner wrote:
> Thus spake Pierre Habouzit (madcoder@...ian.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.

  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@...ian.org
OOO                                                http://www.madism.org

Content of type "application/pgp-signature" skipped

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