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

Re: CVE-2016-8685 in potrace



Hi Ben, Ola,

> This seems to be a correct optimisation.  Overflow/underflow on signed
> arithmetic has undefined behaviour, therefore standard C code will not
> allow it to happen and the compiler may rely on that.  If the code does
> actually cause an overflow, literally anything can result.
> 
> Thankfully gcc does have an option to support code that relies on
> two's-complement wrapping behaviour on signed arithmetic, which is
> -fwrapv.  See also the -fno-strict-overflow option.

You're right ! Compiling with -fwrapv fixes the problem.

Alternatively we could use the attached patch that also seems to fix the
problem.

This patch should be sufficient because in this case an integer overflow can
only occur if there is a x with

 * x < bm->w
 
and 
 
 * x + BM_WORDBITS > INT_MAX
 
thus only if bm->w > INT_MAX - BM_WORDBITS.

I don't know which solution is the best, but the second solution is probably
better for future maintainance.

Cheers,
 Hugo

-- 
             Hugo Lefeuvre (hle)    |    www.owl.eu.com
4096/ ACB7 B67F 197F 9B32 1533 431C AC90 AC3E C524 065E
--- a/src/decompose.c	2017-04-04 10:58:37.436084109 +0200
+++ b/src/decompose.c	2017-04-04 11:22:02.957825883 +0200
@@ -428,6 +428,11 @@
 
   x0 = (*xp) & ~(BM_WORDBITS-1);
 
+  /* check value of bm->w to avoid integer overflow of x in the loop. */
+  if( bm->w >= INT_MAX - BM_WORDBITS ) {
+    return 1;
+  }
+
   for (y=*yp; y>=0; y--) {
     for (x=x0; x<bm->w; x+=BM_WORDBITS) {
       if (*bm_index(bm, x, y)) {

Attachment: signature.asc
Description: PGP signature


Reply to: