Hi!

Frank Küster [2005-12-08 15:54 +0100]:
> Martin Pitt <[EMAIL PROTECTED]> wrote:
> 
> > -      img.tiles = (JPXTile *)gmalloc(img.nXTiles * img.nYTiles *
> > -                                sizeof(JPXTile));
> > +      nTiles = img.nXTiles * img.nYTiles;
> > +      // check for overflow before allocating memory
> > +      if (nTiles == 0 || nTiles / img.nXTiles != img.nYTiles) {
> > +   error(getPos(), "Bad tile count in JPX SIZ marker segment");
> > +   return gFalse;
> > +      }
> > +      img.tiles = (JPXTile *)gmalloc(nTiles * sizeof(JPXTile));
> >
> > gmalloc does a multiplication which is not checked for integer
> > overflows. xpdf uses gmallocn() which does that check.
> 
> xpdf has gmallocn only since 3.01, but tetex-bin uses 3.00.  I wouldn't
> want to update parts of the code, or all of it to 3.01, without
> understanding the differences.  On the other hand, maybe the xpdf code
> in tetex-bin has *more* unchecked buffer overflows exactly because it
> does not yet use gmallocn...

Possibly. gmallocn() is just a shallow wrapper around gmalloc() with
integer overflow checking, so it's not a big deal.

> Would 
> 
>       if (nTiles >= INT_MAX / sizeof(JPXTile) {
>       error(getPos(), "Bad tile count in JPX SIZ marker segment");
>       return gFalse;
> 
> be okay?

This is the standard way of checking for multiplicative overflows,
that looks fine.

Martin

-- 
Martin Pitt        http://www.piware.de
Ubuntu Developer   http://www.ubuntu.com
Debian Developer   http://www.debian.org

In a world without walls and fences, who needs Windows and Gates?

Attachment: signature.asc
Description: Digital signature

Reply via email to