Bug#342292: Fwd: Re: [vendor-sec] xpdf update - patch wrong?

[Frank Küster <frank@kuesterei.ch>]

Martin Pitt <mpitt@debian.org> 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