Comment 40 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!

Frank Küster [2005-12-09 11:09 +0100]:
> Martin Pitt <email address hidden> wrote:
>
> > 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 what if nObjs * objSize is larger than fits into an int?

Handling this case is the sole purpose of this gmallocn() wrapper.

Let N be the product of nObjs * objSize in the naturals.

- For valid (small) positive values of nObjs, n == N and the division is ok.

- For invalid (big) positive values of nObjs which, when multiplied with nObjs
  overflow an int, we have two cases:

  * n == N mod 2^31 (i. e. product overflows into the positive half of int space)
    => n < N
    => n/objSize < N/objSize
    => n/objSize < nObjs
    => n/objSize != nObjs
    => check fails.

  * n < 0
    => n/objSize < 0
    => since by assumption nObjs > 0: n/objSize != nObjs
    => check fails.

As I already said, the function will cause trouble (allocating
insanely amounts of memory, but probably not an overflow) for negative
nObjs. Thus, the function should either use unsigneds, or at least
check that nObjs and objSize > 0.

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?