Comment 36 for bug 26650

Message-ID: <email address hidden>
Date: Fri, 9 Dec 2005 10:17:51 +0100
From: Martin Pitt <email address hidden>
To: Frank =?iso-8859-1?Q?K=FCster?= <email address hidden>
Cc: Martin Pitt <email address hidden>, Martin Pitt <email address hidden>, <email address hidden>
Subject: Re: Bug#342292: Fwd: Re: [vendor-sec] xpdf update - patch wrong?

--7LkOrbQMr4cezO2T
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi!

Frank K=FCster [2005-12-08 15:54 +0100]:
> Martin Pitt <email address hidden> wrote:
>=20
> > - img.tiles =3D (JPXTile *)gmalloc(img.nXTiles * img.nYTiles *
> > - sizeof(JPXTile));
> > + nTiles =3D img.nXTiles * img.nYTiles;
> > + // check for overflow before allocating memory
> > + if (nTiles =3D=3D 0 || nTiles / img.nXTiles !=3D img.nYTiles) {
> > + error(getPos(), "Bad tile count in JPX SIZ marker segment");
> > + return gFalse;
> > + }
> > + img.tiles =3D (JPXTile *)gmalloc(nTiles * sizeof(JPXTile));
> >
> > gmalloc does a multiplication which is not checked for integer
> > overflows. xpdf uses gmallocn() which does that check.
>=20
> 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=20
>=20
> if (nTiles >=3D INT_MAX / sizeof(JPXTile) {
> error(getPos(), "Bad tile count in JPX SIZ marker segment");
> return gFalse;
>=20
> be okay?

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

Martin

--=20
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?

--7LkOrbQMr4cezO2T
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQFDmUu/DecnbV4Fd/IRAlO5AKCNMZgei17LJra3eFPATfqWOWv8SwCggVxa
c3P8msl8pXRk7p4AajZOgDQ=
=fSFz
-----END PGP SIGNATURE-----

--7LkOrbQMr4cezO2T--