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

[Martin Schulze <joey@infodrom.org>]

Martin Pitt wrote:
> > > After discovering that the same flawed multiplication is also present
> > > in upstream's other two patches, I decided to completely rework the
> > > patch.
> > >
> > > I attach the debdiff with separated out changelog. Florian, maybe you
> > > can peer-review the patch?
> > 
> > Martin and Florian,  Joey Schulze also sent a "fixed" patch to the bug,
> > see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=342292;msg=131
> > 
> > Would you be so kind and review it?
> 
> Sorry for the delay, lots of private stuff to do on the weekend.
> 
> +   nVals = width * nComps;
> ++  totalBits = nVals * nBits;
> ++  if (totalBits == 0 ||
> ++      (totalBits / nBits) / nComps != width ||
> ++      totalBits + 7 < 0) {
> ++    return;
> ++  }
> 
> Please do not use this part of Joey's patch. As already disdussed,
> this way of checking a multiplication overflow is unreliable. Please
> use the var1 >= INT_MAX/var2 approach, which is the 'standard way' and
> avoids integer overflows.

Sorry, that slipped through from one of the load of different patches.

It's not that problematic, the line

> ++      (totalBits / nBits) / nComps != width ||

might be optimised by the compiler, but it's not a problem since the
proper check is above the code (at least in my local copy):

+      nComps >= INT_MAX/nBits ||
+      width >= INT_MAX/nComps/nBits) {

Regards,

	Joey

-- 
If nothing changes, everything will remain the same.  -- Barne's Law

Please always Cc to me when replying to me on the lists.