Comment 33 for bug 26650

Hi!

Frank Küster [2005-12-08 15:54 +0100]:
> Martin Pitt <email address hidden> 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?