[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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



Hi!

Frank Küster [2005-12-09 11:09 +0100]:
> Martin Pitt <mpitt@debian.org> wrote:
> 
> > Hi Florian, hi Frank!
> >
> > Frank Küster [2005-12-08 22:55 +0100]:
> >> Florian Weimer <fw@deneb.enyo.de> 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?

Attachment: signature.asc
Description: Digital signature


Reply to: