Re: string.unpack segfaulting due to integer overflow and mixed sign comparisons

[William Ahern <william@...>]

On Wed, Jun 27, 2012 at 12:42:16PM +0100, Joshua Phillips wrote:
> On Wed, Jun 27, 2012 at 09:14:23AM +0100, Chris Emerson wrote:
> > On Tue, Jun 26, 2012 at 03:51:43PM -0700, Sam Roberts wrote:
> > > Index: pack/lpack.c
> > > ===================================================================
> > > --- pack/lpack.c        (revision 27854)
> > > +++ pack/lpack.c        (working copy)
> > > @@ -129,7 +129,7 @@
> > >     case OP_STRING:
> > >     {
> > >      ++N;
> > > -    if (i+N>len) goto done;
> > > +    if (i+N < i || i+N>len) goto done;
> > 
> > I don't think this is right - in C overflow of signed integers is undefined
> > behaviour.  The C compiler can (and some now do) assume that "i+N < i"
> > (with N positive) can't happen, and that test can be optimised out.
> > 
> > I haven't looked at it in enough detail to provide the right fix, sorry.
> > :-)
> 
> You're right, you can't do the check after the overflow, because the
> overflow itself is undefined behaviour.
> This document goes into more detail (and shows how hard it is to use C
> safely!): http://www.fefe.de/intof.html

It's not just C, it's any language with fixed-width value representations.
C, however, is far less lenient with the consequences. Or rather, the
consequences are quicker. (Most such bugs in Java, Perl, Lua, etc still
linger around; they're just rarely triggered.)

But that's a bonus. When you know--as you should--that there's an ax hanging
over your head, you tend to be a little more cautious.