Comment 18 for bug 26650

Revision history for this message
In , Frank Küster (frank-kuesterei) wrote : Re: Bug#342292: Fwd: Re: [vendor-sec] xpdf update - patch wrong?

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

Would

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

be okay?

Regards, Frank

--
Frank Küster
Inst. f. Biochemie der Univ. Zürich
Debian Developer