Merge pull request #255 from libtom/hardening

sjaeckel · web-flow · commit 235e831db0c4 · 2019-05-12T11:52:48.000+02:00
hardening: add MP_ZERO_BUFFER, MP_ZERO_DIGITS
diff --git a/astylerc b/astylerc
@@ -4,6 +4,9 @@
 # usage:
 #       astyle --options=astylerc *.[ch]
 
+# Do not create backup, annonying in the times of git
+suffix=none
+
 ## Bracket Style Options
 style=kr
 
diff --git a/bn_mp_add_d.c b/bn_mp_add_d.c
@@ -85,9 +85,7 @@ int mp_add_d(const mp_int *a, mp_digit b, mp_int *c)
    c->sign = MP_ZPOS;
 
    /* now zero to oldused */
-   while (ix++ < oldused) {
-      *tmpc++ = 0;
-   }
+   MP_ZERO_DIGITS(tmpc, oldused - ix);
    mp_clamp(c);
 
    return MP_OKAY;
diff --git a/bn_mp_and.c b/bn_mp_and.c
@@ -29,9 +29,7 @@ int mp_and(const mp_int *a, const mp_int *b, mp_int *c)
    }
 
    /* zero digits above the last from the smallest mp_int */
-   for (; ix < t.used; ix++) {
-      t.dp[ix] = 0;
-   }
+   MP_ZERO_DIGITS(t.dp + ix, t.used - ix);
 
    mp_clamp(&t);
    mp_exch(c, &t);
diff --git a/bn_mp_clear.c b/bn_mp_clear.c
@@ -6,17 +6,10 @@
 /* clear one (frees)  */
 void mp_clear(mp_int *a)
 {
-   int i;
-
    /* only do anything if a hasn't been freed previously */
    if (a->dp != NULL) {
-      /* first zero the digits */
-      for (i = 0; i < a->used; i++) {
-         a->dp[i] = 0;
-      }
-
       /* free ram */
-      MP_FREE(a->dp, sizeof(mp_digit) * (size_t)a->alloc);
+      MP_FREE_DIGITS(a->dp, a->alloc);
 
       /* reset members to make debugging easier */
       a->dp    = NULL;
diff --git a/bn_mp_copy.c b/bn_mp_copy.c
@@ -38,9 +38,7 @@ int mp_copy(const mp_int *a, mp_int *b)
       }
 
       /* clear high digits */
-      for (; n < b->used; n++) {
-         *tmpb++ = 0;
-      }
+      MP_ZERO_DIGITS(tmpb, b->used - n);
    }
 
    /* copy used count and sign */
diff --git a/bn_mp_div_2.c b/bn_mp_div_2.c
@@ -40,10 +40,7 @@ int mp_div_2(const mp_int *a, mp_int *b)
       }
 
       /* zero excess digits */
-      tmpb = b->dp + b->used;
-      for (x = b->used; x < oldused; x++) {
-         *tmpb++ = 0;
-      }
+      MP_ZERO_DIGITS(b->dp + b->used, oldused - b->used);
    }
    b->sign = a->sign;
    mp_clamp(b);
diff --git a/bn_mp_dr_reduce.c b/bn_mp_dr_reduce.c
@@ -58,9 +58,7 @@ int mp_dr_reduce(mp_int *x, const mp_int *n, mp_digit k)
    *tmpx1++ = mu;
 
    /* zero words above m */
-   for (i = m + 1; i < x->used; i++) {
-      *tmpx1++ = 0;
-   }
+   MP_ZERO_DIGITS(tmpx1, x->used - m - 1);
 
    /* clamp, sub and return */
    mp_clamp(x);
diff --git a/bn_mp_fwrite.c b/bn_mp_fwrite.c
@@ -19,18 +19,18 @@ int mp_fwrite(const mp_int *a, int radix, FILE *stream)
    }
 
    if ((err = mp_toradix(a, buf, radix)) != MP_OKAY) {
-      MP_FREE(buf, len);
+      MP_FREE_BUFFER(buf, (size_t)len);
       return err;
    }
 
    for (x = 0; x < len; x++) {
       if (fputc((int)buf[x], stream) == EOF) {
-         MP_FREE(buf, len);
+         MP_FREE_BUFFER(buf, (size_t)len);
          return MP_VAL;
       }
    }
 
-   MP_FREE(buf, len);
+   MP_FREE_BUFFER(buf, (size_t)len);
    return MP_OKAY;
 }
 #endif
diff --git a/bn_mp_grow.c b/bn_mp_grow.c
@@ -34,9 +34,7 @@ int mp_grow(mp_int *a, int size)
       /* zero excess digits */
       i        = a->alloc;
       a->alloc = size;
-      for (; i < a->alloc; i++) {
-         a->dp[i] = 0;
-      }
+      MP_ZERO_DIGITS(a->dp + i, a->alloc - i);
    }
    return MP_OKAY;
 }
diff --git a/bn_mp_mod_2d.c b/bn_mp_mod_2d.c
@@ -25,9 +25,9 @@ int mp_mod_2d(const mp_int *a, int b, mp_int *c)
    }
 
    /* zero digits above the last digit of the modulus */
-   for (x = (b / MP_DIGIT_BIT) + (((b % MP_DIGIT_BIT) == 0) ? 0 : 1); x < c->used; x++) {
-      c->dp[x] = 0;
-   }
+   x = (b / MP_DIGIT_BIT) + (((b % MP_DIGIT_BIT) == 0) ? 0 : 1);
+   MP_ZERO_DIGITS(c->dp + x, c->used - x);
+
    /* clear the digit that is not completely outside/inside the modulus */
    c->dp[b / MP_DIGIT_BIT] &=
       ((mp_digit)1 << (mp_digit)(b % MP_DIGIT_BIT)) - (mp_digit)1;
diff --git a/bn_mp_mul_2.c b/bn_mp_mul_2.c
@@ -55,10 +55,7 @@ int mp_mul_2(const mp_int *a, mp_int *b)
       /* now zero any excess digits on the destination
        * that we didn't write to
        */
-      tmpb = b->dp + b->used;
-      for (x = b->used; x < oldused; x++) {
-         *tmpb++ = 0;
-      }
+      MP_ZERO_DIGITS(b->dp + b->used, oldused - b->used);
    }
    b->sign = a->sign;
    return MP_OKAY;
diff --git a/bn_mp_mul_d.c b/bn_mp_mul_d.c
@@ -49,9 +49,7 @@ int mp_mul_d(const mp_int *a, mp_digit b, mp_int *c)
    ++ix;
 
    /* now zero digits above the top */
-   while (ix++ < olduse) {
-      *tmpc++ = 0;
-   }
+   MP_ZERO_DIGITS(tmpc, olduse - ix);
 
    /* set used count */
    c->used = a->used + 1;
diff --git a/bn_mp_prime_rand.c b/bn_mp_prime_rand.c
@@ -114,7 +114,7 @@ static int s_mp_prime_random_ex(mp_int *a, int t, int size, int flags, private_m
 
    err = MP_OKAY;
 error:
-   MP_FREE(tmp, bsize);
+   MP_FREE_BUFFER(tmp, (size_t)bsize);
    return err;
 }
 
diff --git a/bn_mp_sub_d.c b/bn_mp_sub_d.c
@@ -67,9 +67,8 @@ int mp_sub_d(const mp_int *a, mp_digit b, mp_int *c)
    }
 
    /* zero excess digits */
-   while (ix++ < oldused) {
-      *tmpc++ = 0;
-   }
+   MP_ZERO_DIGITS(tmpc, oldused - ix);
+
    mp_clamp(c);
    return MP_OKAY;
 }
diff --git a/bn_mp_zero.c b/bn_mp_zero.c
@@ -6,15 +6,8 @@
 /* set to zero */
 void mp_zero(mp_int *a)
 {
-   int       n;
-   mp_digit *tmp;
-
    a->sign = MP_ZPOS;
    a->used = 0;
-
-   tmp = a->dp;
-   for (n = 0; n < a->alloc; n++) {
-      *tmp++ = 0;
-   }
+   MP_ZERO_DIGITS(a->dp, a->alloc);
 }
 #endif
diff --git a/bn_s_mp_add.c b/bn_s_mp_add.c
@@ -81,9 +81,7 @@ int s_mp_add(const mp_int *a, const mp_int *b, mp_int *c)
       *tmpc++ = u;
 
       /* clear digits above oldused */
-      for (i = c->used; i < olduse; i++) {
-         *tmpc++ = 0;
-      }
+      MP_ZERO_DIGITS(tmpc, olduse - c->used);
    }
 
    mp_clamp(c);
diff --git a/bn_s_mp_montgomery_reduce_fast.c b/bn_s_mp_montgomery_reduce_fast.c
@@ -49,8 +49,8 @@ int s_mp_montgomery_reduce_fast(mp_int *x, const mp_int *n, mp_digit rho)
       }
 
       /* zero the high words of W[a->used..m->used*2] */
-      for (; ix < ((n->used * 2) + 1); ix++) {
-         *_W++ = 0;
+      if (ix < ((n->used * 2) + 1)) {
+         MP_ZERO_BUFFER(_W, sizeof(mp_word) * (size_t)(((n->used * 2) + 1) - ix));
       }
    }
 
@@ -142,9 +142,7 @@ int s_mp_montgomery_reduce_fast(mp_int *x, const mp_int *n, mp_digit rho)
       /* zero oldused digits, if the input a was larger than
        * m->used+1 we'll have to clear the digits
        */
-      for (; ix < olduse; ix++) {
-         *tmpx++ = 0;
-      }
+      MP_ZERO_DIGITS(tmpx, olduse - ix);
    }
 
    /* set the max used and clamp */
diff --git a/bn_s_mp_mul_digs_fast.c b/bn_s_mp_mul_digs_fast.c
@@ -81,9 +81,7 @@ int s_mp_mul_digs_fast(const mp_int *a, const mp_int *b, mp_int *c, int digs)
       }
 
       /* clear unused digits [that existed in the old copy of c] */
-      for (; ix < olduse; ix++) {
-         *tmpc++ = 0;
-      }
+      MP_ZERO_DIGITS(tmpc, olduse - ix);
    }
    mp_clamp(c);
    return MP_OKAY;
diff --git a/bn_s_mp_mul_high_digs_fast.c b/bn_s_mp_mul_high_digs_fast.c
@@ -72,9 +72,7 @@ int s_mp_mul_high_digs_fast(const mp_int *a, const mp_int *b, mp_int *c, int dig
       }
 
       /* clear unused digits [that existed in the old copy of c] */
-      for (; ix < olduse; ix++) {
-         *tmpc++ = 0;
-      }
+      MP_ZERO_DIGITS(tmpc, olduse - ix);
    }
    mp_clamp(c);
    return MP_OKAY;
diff --git a/bn_s_mp_sqr_fast.c b/bn_s_mp_sqr_fast.c
@@ -88,9 +88,7 @@ int s_mp_sqr_fast(const mp_int *a, mp_int *b)
       }
 
       /* clear unused digits [that existed in the old copy of c] */
-      for (; ix < olduse; ix++) {
-         *tmpb++ = 0;
-      }
+      MP_ZERO_DIGITS(tmpb, olduse - ix);
    }
    mp_clamp(b);
    return MP_OKAY;
diff --git a/bn_s_mp_sub.c b/bn_s_mp_sub.c
@@ -60,9 +60,7 @@ int s_mp_sub(const mp_int *a, const mp_int *b, mp_int *c)
       }
 
       /* clear digits above used (since we may not have grown result above) */
-      for (i = c->used; i < olduse; i++) {
-         *tmpc++ = 0;
-      }
+      MP_ZERO_DIGITS(tmpc, olduse - c->used);
    }
 
    mp_clamp(c);
diff --git a/tommath_private.h b/tommath_private.h
@@ -10,6 +10,42 @@
 extern "C" {
 #endif
 
+/* Hardening libtommath
+ * --------------------
+ *
+ * By default memory is zeroed before calling
+ * MP_FREE to avoid leaking data. This is good
+ * practice in cryptographical applications.
+ *
+ * Note however that memory allocators used
+ * in cryptographical applications can often
+ * be configured by itself to clear memory,
+ * rendering the clearing in tommath unnecessary.
+ * See for example https://github.com/GrapheneOS/hardened_malloc
+ * and the option CONFIG_ZERO_ON_FREE.
+ *
+ * Furthermore there are applications which
+ * value performance more and want this
+ * feature to be disabled. For such applications
+ * define MP_NO_ZERO_ON_FREE during compilation.
+ */
+#ifdef MP_NO_ZERO_ON_FREE
+#  define MP_FREE_BUFFER(mem, size)   MP_FREE((mem), (size))
+#  define MP_FREE_DIGITS(mem, digits) MP_FREE((mem), sizeof (mp_digit) * (digits))
+#else
+#  define MP_FREE_BUFFER(mem, size)   do { size_t fs_ = (size); void* fm_ = (mem); if (fm_) { MP_ZERO_BUFFER(fm_, fs_); MP_FREE(fm_, fs_); } } while (0)
+#  define MP_FREE_DIGITS(mem, digits) do { int fd_ = (digits); void* fm_ = (mem); if (fm_) { MP_ZERO_BUFFER(fm_, sizeof (mp_digit) * (size_t)fd_); MP_FREE(fm_, sizeof (mp_digit) * (size_t)fd_); } } while (0)
+#endif
+
+#ifdef MP_USE_MEMSET
+#  include <string.h>
+#  define MP_ZERO_BUFFER(mem, size)   memset((mem), 0, (size))
+#  define MP_ZERO_DIGITS(mem, digits) do { int zd_ = (digits); if (zd_ > 0) { memset((mem), 0, sizeof (mp_digit) * (size_t)zd_); } } while (0)
+#else
+#  define MP_ZERO_BUFFER(mem, size)   do { size_t zs_ = (size); char* zm_ = (char*)(mem); while (zs_-- > 0) { *zm_++ = 0; } } while (0)
+#  define MP_ZERO_DIGITS(mem, digits) do { int zd_ = (digits); mp_digit* zm_ = (mem); while (zd_-- > 0) { *zm_++ = 0; } } while (0)
+#endif
+
 /* Tunable cutoffs
  * ---------------
  *
@@ -43,8 +79,8 @@ extern "C" {
 /* default to libc stuff */
 #   include <stdlib.h>
 #   define MP_MALLOC(size)                   malloc(size)
-#   define MP_REALLOC(mem, oldsize, newsize) realloc(mem, newsize)
-#   define MP_CALLOC(nmemb, size)            calloc(nmemb, size)
+#   define MP_REALLOC(mem, oldsize, newsize) realloc((mem), (newsize))
+#   define MP_CALLOC(nmemb, size)            calloc((nmemb), (size))
 #   define MP_FREE(mem, size)                free(mem)
 #else
 /* prototypes for our heap functions */

Original file line number	Diff line number	Diff line change
`@@ -29,9 +29,7 @@ int mp_and(const mp_int a, const mp_int b, mp_int *c)`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`/* zero digits above the last from the smallest mp_int */`
`32`		`- for (; ix < t.used; ix++) {`
`33`		`- t.dp[ix] = 0;`
`34`		`- }`
	`32`	`+ MP_ZERO_DIGITS(t.dp + ix, t.used - ix);`
`35`	`33`
`36`	`34`	`mp_clamp(&t);`
`37`	`35`	`mp_exch(c, &t);`
Original file line number	Diff line number	Diff line change
`@@ -38,9 +38,7 @@ int mp_copy(const mp_int a, mp_int b)`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`/* clear high digits */`
`41`		`- for (; n < b->used; n++) {`
`42`		`- *tmpb++ = 0;`
`43`		`- }`
	`41`	`+ MP_ZERO_DIGITS(tmpb, b->used - n);`
`44`	`42`	`}`
`45`	`43`
`46`	`44`	`/* copy used count and sign */`
Original file line number	Diff line number	Diff line change
`@@ -40,10 +40,7 @@ int mp_div_2(const mp_int a, mp_int b)`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`/* zero excess digits */`
`43`		`- tmpb = b->dp + b->used;`
`44`		`- for (x = b->used; x < oldused; x++) {`
`45`		`- *tmpb++ = 0;`
`46`		`- }`
	`43`	`+ MP_ZERO_DIGITS(b->dp + b->used, oldused - b->used);`
`47`	`44`	`}`
`48`	`45`	`b->sign = a->sign;`
`49`	`46`	`mp_clamp(b);`
Original file line number	Diff line number	Diff line change
`@@ -19,18 +19,18 @@ int mp_fwrite(const mp_int a, int radix, FILE stream)`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`if ((err = mp_toradix(a, buf, radix)) != MP_OKAY) {`
`22`		`- MP_FREE(buf, len);`
	`22`	`+ MP_FREE_BUFFER(buf, (size_t)len);`
`23`	`23`	`return err;`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`for (x = 0; x < len; x++) {`
`27`	`27`	`if (fputc((int)buf[x], stream) == EOF) {`
`28`		`- MP_FREE(buf, len);`
	`28`	`+ MP_FREE_BUFFER(buf, (size_t)len);`
`29`	`29`	`return MP_VAL;`
`30`	`30`	`}`
`31`	`31`	`}`
`32`	`32`
`33`		`- MP_FREE(buf, len);`
	`33`	`+ MP_FREE_BUFFER(buf, (size_t)len);`
`34`	`34`	`return MP_OKAY;`
`35`	`35`	`}`
`36`	`36`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -34,9 +34,7 @@ int mp_grow(mp_int *a, int size)`
`34`	`34`	`/* zero excess digits */`
`35`	`35`	`i = a->alloc;`
`36`	`36`	`a->alloc = size;`
`37`		`- for (; i < a->alloc; i++) {`
`38`		`- a->dp[i] = 0;`
`39`		`- }`
	`37`	`+ MP_ZERO_DIGITS(a->dp + i, a->alloc - i);`
`40`	`38`	`}`
`41`	`39`	`return MP_OKAY;`
`42`	`40`	`}`