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: <20070111151502.GD20797@mad.intersec.eu>
Date: Thu, 11 Jan 2007 16:15:02 +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: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@...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.

  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@...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/