Comment 35 for bug 26650

Revision history for this message
In , Martin Pitt (pitti) wrote : Re: Bug#342292: Fwd: Re: [vendor-sec] xpdf update - patch wrong?

Hi Florian, hi Frank!

Frank Küster [2005-12-08 22:55 +0100]:
> Florian Weimer <email address hidden> wrote:
> > By the way, the gmallocn function suffers from undefined integer
> > overflow, too:
> >
> > void *gmallocn(int nObjs, int objSize) {
> > int n;
> >
> > n = nObjs * objSize;
> > if (objSize == 0 || n / objSize != nObjs) {
> > fprintf(stderr, "Bogus memory allocation size\n");
> > exit(1);
> > }
> > return gmalloc(n);
> > }
>
> What's the problem here? That the value in "n" is undefined, and
> therefore the comparison n / objSize != nObjs is undefined, too?

n is not 'undefined' here. For every given nObjs and objSize input, it
always gets the same well-defined value.

We can assume that objSize is a small positive number, since it is not
user defined (just a sizeof value). The function works correctly for
positive number of nObjs (both valid and invalid), but there is a
corner case for negative nOjbs. Since gmalloc() takes a size_t
(unsigned), in most cases gmalloc() will allocate more memory than
required for a negative argument. However, when n is exactly -2^31 you
could see an off-by-one memory allocation error.

Indeed the function should completely be written using unsigned
arithmetics, otherwise your head will just explode.

Florian, is that what you meant?

Thanks,

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?