From 2f6c8019114743e46a741fc6a01f9c6600ccdb5d Mon Sep 17 00:00:00 2001
From: Gregory Maxwell <greg@xiph.org>
Date: Thu, 14 Aug 2014 06:58:57 -0700
Subject: [PATCH] Try to not leave secret data on the stack or heap.

This makes a basic effort and has not been audited.
Doesn't appear to have a measurable performance impact on bench.

It also adds a secp256k1_num_free to secp256k1_ecdsa_pubkey_create.
---
 src/ecdsa_impl.h       |  3 +++
 src/ecmult_impl.h      |  8 ++++++--
 src/field_10x26_impl.h | 10 ++++++++++
 src/field_5x52_impl.h  | 10 ++++++++++
 src/field_gmp_impl.h   |  5 +++++
 src/group.h            |  7 +++++++
 src/group_impl.h       | 13 +++++++++++++
 src/num.h              |  3 +++
 src/num_gmp_impl.h     | 13 ++++++++++++-
 src/num_openssl_impl.h |  4 ++++
 src/secp256k1.c        | 11 +++++++++++
 11 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h
index 2792e3388ad..71762abebe4 100644
--- a/src/ecdsa_impl.h
+++ b/src/ecdsa_impl.h
@@ -186,7 +186,10 @@ int static secp256k1_ecdsa_sig_sign(secp256k1_ecdsa_sig_t *sig, const secp256k1_
     secp256k1_num_mod(&n, &c->order);
     secp256k1_num_mod_inverse(&sig->s, nonce, &c->order);
     secp256k1_num_mod_mul(&sig->s, &sig->s, &n, &c->order);
+    secp256k1_num_clear(&n);
     secp256k1_num_free(&n);
+    secp256k1_gej_clear(&rp);
+    secp256k1_ge_clear(&r);
     if (secp256k1_num_is_zero(&sig->s))
         return 0;
     if (secp256k1_num_cmp(&sig->s, &c->half_order) > 0) {
diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h
index 2dba4b8d46d..10e8e455778 100644
--- a/src/ecmult_impl.h
+++ b/src/ecmult_impl.h
@@ -190,13 +190,17 @@ void static secp256k1_ecmult_gen(secp256k1_gej_t *r, const secp256k1_num_t *gn)
     secp256k1_num_copy(&n, gn);
     const secp256k1_ecmult_consts_t *c = secp256k1_ecmult_consts;
     secp256k1_gej_set_infinity(r);
+    secp256k1_ge_t add;
+    int bits;
     for (int j=0; j<64; j++) {
-        secp256k1_ge_t add;
-        int bits = secp256k1_num_shift(&n, 4);
+        bits = secp256k1_num_shift(&n, 4);
         for (int k=0; k<sizeof(secp256k1_ge_t); k++)
             ((unsigned char*)(&add))[k] = c->prec[j][k][bits];
         secp256k1_gej_add_ge(r, r, &add);
     }
+    bits = 0;
+    secp256k1_ge_clear(&add);
+    secp256k1_num_clear(&n);
     secp256k1_num_free(&n);
     secp256k1_gej_add_ge(r, r, &c->fin);
 }
diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h
index 734116697fb..4818131818e 100644
--- a/src/field_10x26_impl.h
+++ b/src/field_10x26_impl.h
@@ -92,6 +92,16 @@ int static inline secp256k1_fe_is_odd(const secp256k1_fe_t *a) {
     return a->n[0] & 1;
 }
 
+void static inline secp256k1_fe_clear(secp256k1_fe_t *a) {
+#ifdef VERIFY
+    a->normalized = 0;
+    a->magnitude = 0;
+#endif
+    for (int i=0; i<10; i++) {
+        a->n[i] = 0;
+    }
+}
+
 // TODO: not constant time!
 int static inline secp256k1_fe_equal(const secp256k1_fe_t *a, const secp256k1_fe_t *b) {
 #ifdef VERIFY
diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h
index ccebdab078a..b2c924af319 100644
--- a/src/field_5x52_impl.h
+++ b/src/field_5x52_impl.h
@@ -124,6 +124,16 @@ int static inline secp256k1_fe_is_odd(const secp256k1_fe_t *a) {
     return a->n[0] & 1;
 }
 
+void static inline secp256k1_fe_clear(secp256k1_fe_t *a) {
+#ifdef VERIFY
+    a->magnitude = 0;
+    a->normalized = 0;
+#endif
+    for (int i=0; i<5; i++) {
+        a->n[i] = 0;
+    }
+}
+
 // TODO: not constant time!
 int static inline secp256k1_fe_equal(const secp256k1_fe_t *a, const secp256k1_fe_t *b) {
 #ifdef VERIFY
diff --git a/src/field_gmp_impl.h b/src/field_gmp_impl.h
index 8d2167d0af3..d97cfa73411 100644
--- a/src/field_gmp_impl.h
+++ b/src/field_gmp_impl.h
@@ -50,6 +50,11 @@ void static inline secp256k1_fe_set_int(secp256k1_fe_t *r, int a) {
         r->n[i] = 0;
 }
 
+void static inline secp256k1_fe_clear(secp256k1_fe_t *r) {
+    for (int i=0; i<FIELD_LIMBS+1; i++)
+        r->n[i] = 0;
+}
+
 int static inline secp256k1_fe_is_zero(const secp256k1_fe_t *a) {
     int ret = 1;
     for (int i=0; i<FIELD_LIMBS+1; i++)
diff --git a/src/group.h b/src/group.h
index bf1bd3b7eb9..aea5658c121 100644
--- a/src/group.h
+++ b/src/group.h
@@ -112,4 +112,11 @@ void static secp256k1_gej_mul_lambda(secp256k1_gej_t *r, const secp256k1_gej_t *
 void static secp256k1_gej_split_exp(secp256k1_num_t *r1, secp256k1_num_t *r2, const secp256k1_num_t *a);
 #endif
 
+/** Clear a secp256k1_gej_t to prevent leaking sensitive information. */
+void static secp256k1_gej_clear(secp256k1_gej_t *r);
+
+/** Clear a secp256k1_ge_t to prevent leaking sensitive information. */
+void static secp256k1_ge_clear(secp256k1_ge_t *r);
+
+
 #endif
diff --git a/src/group_impl.h b/src/group_impl.h
index 30a6cf8a951..8951882593c 100644
--- a/src/group_impl.h
+++ b/src/group_impl.h
@@ -102,6 +102,19 @@ void static secp256k1_gej_set_xy(secp256k1_gej_t *r, const secp256k1_fe_t *x, co
     secp256k1_fe_set_int(&r->z, 1);
 }
 
+void static secp256k1_gej_clear(secp256k1_gej_t *r) {
+    r->infinity = 0;
+    secp256k1_fe_clear(&r->x);
+    secp256k1_fe_clear(&r->y);
+    secp256k1_fe_clear(&r->z);
+}
+
+void static secp256k1_ge_clear(secp256k1_ge_t *r) {
+    r->infinity = 0;
+    secp256k1_fe_clear(&r->x);
+    secp256k1_fe_clear(&r->y);
+}
+
 int static secp256k1_ge_set_xo(secp256k1_ge_t *r, const secp256k1_fe_t *x, int odd) {
     r->x = *x;
     secp256k1_fe_t x2; secp256k1_fe_sqr(&x2, x);
diff --git a/src/num.h b/src/num.h
index 596a1226c2d..83ec497fbb8 100644
--- a/src/num.h
+++ b/src/num.h
@@ -20,6 +20,9 @@
 /** Initialize a number. */
 void static secp256k1_num_init(secp256k1_num_t *r);
 
+/** Clear a number to prevent the leak of sensitive data. */
+void static secp256k1_num_clear(secp256k1_num_t *r);
+
 /** Free a number. */
 void static secp256k1_num_free(secp256k1_num_t *r);
 
diff --git a/src/num_gmp_impl.h b/src/num_gmp_impl.h
index 75ef9dcf682..d8f7890e76f 100644
--- a/src/num_gmp_impl.h
+++ b/src/num_gmp_impl.h
@@ -26,6 +26,10 @@ void static secp256k1_num_init(secp256k1_num_t *r) {
     r->data[0] = 0;
 }
 
+void static secp256k1_num_clear(secp256k1_num_t *r) {
+    memset(r, 0, sizeof(*r));
+}
+
 void static secp256k1_num_free(secp256k1_num_t *r) {
 }
 
@@ -54,8 +58,10 @@ void static secp256k1_num_get_bin(unsigned char *r, unsigned int rlen, const sec
     while (shift < len && tmp[shift] == 0) shift++;
     assert(len-shift <= rlen);
     memset(r, 0, rlen - len + shift);
-    if (len > shift)
+    if (len > shift) {
         memcpy(r + rlen - len + shift, tmp + shift, len - shift);
+    }
+    memset(tmp, 0, sizeof(tmp));
 }
 
 void static secp256k1_num_set_bin(secp256k1_num_t *r, const unsigned char *a, unsigned int alen) {
@@ -97,6 +103,7 @@ void static secp256k1_num_mod(secp256k1_num_t *r, const secp256k1_num_t *m) {
     if (r->limbs >= m->limbs) {
         mp_limb_t t[2*NUM_LIMBS];
         mpn_tdiv_qr(t, r->data, 0, r->data, r->limbs, m->data, m->limbs);
+        memset(t, 0, sizeof(t));
         r->limbs = m->limbs;
         while (r->limbs > 1 && r->data[r->limbs-1]==0) r->limbs--;
     }
@@ -141,6 +148,9 @@ void static secp256k1_num_mod_inverse(secp256k1_num_t *r, const secp256k1_num_t
     } else {
         r->limbs = sn;
     }
+    memset(g, 0, sizeof(g));
+    memset(u, 0, sizeof(u));
+    memset(v, 0, sizeof(v));
 }
 
 int static secp256k1_num_is_zero(const secp256k1_num_t *a) {
@@ -213,6 +223,7 @@ void static secp256k1_num_mul(secp256k1_num_t *r, const secp256k1_num_t *a, cons
     assert(r->limbs <= 2*NUM_LIMBS);
     mpn_copyi(r->data, tmp, r->limbs);
     r->neg = a->neg ^ b->neg;
+    memset(tmp, 0, sizeof(tmp));
 }
 
 void static secp256k1_num_div(secp256k1_num_t *r, const secp256k1_num_t *a, const secp256k1_num_t *b) {
diff --git a/src/num_openssl_impl.h b/src/num_openssl_impl.h
index 948b2e7a2b0..64e6e836711 100644
--- a/src/num_openssl_impl.h
+++ b/src/num_openssl_impl.h
@@ -21,6 +21,10 @@ void static secp256k1_num_free(secp256k1_num_t *r) {
     BN_free(&r->bn);
 }
 
+void static secp256k1_num_clear(secp256k1_num_t *r) {
+    BN_clear(&r->bn);
+}
+
 void static secp256k1_num_copy(secp256k1_num_t *r, const secp256k1_num_t *a) {
     BN_copy(&r->bn, &a->bn);
 }
diff --git a/src/secp256k1.c b/src/secp256k1.c
index 23e058b3b70..115f140a32e 100644
--- a/src/secp256k1.c
+++ b/src/secp256k1.c
@@ -68,6 +68,9 @@ int secp256k1_ecdsa_sign(const unsigned char *message, int messagelen, unsigned
         secp256k1_ecdsa_sig_serialize(signature, signaturelen, &sig);
     }
     secp256k1_ecdsa_sig_free(&sig);
+    secp256k1_num_clear(&msg);
+    secp256k1_num_clear(&non);
+    secp256k1_num_clear(&sec);
     secp256k1_num_free(&msg);
     secp256k1_num_free(&non);
     secp256k1_num_free(&sec);
@@ -94,6 +97,9 @@ int secp256k1_ecdsa_sign_compact(const unsigned char *message, int messagelen, u
         secp256k1_num_get_bin(sig64 + 32, 32, &sig.s);
     }
     secp256k1_ecdsa_sig_free(&sig);
+    secp256k1_num_clear(&msg);
+    secp256k1_num_clear(&non);
+    secp256k1_num_clear(&sec);
     secp256k1_num_free(&msg);
     secp256k1_num_free(&non);
     secp256k1_num_free(&sec);
@@ -126,6 +132,7 @@ int secp256k1_ecdsa_seckey_verify(const unsigned char *seckey) {
     secp256k1_num_set_bin(&sec, seckey, 32);
     int ret = !secp256k1_num_is_zero(&sec) &&
               (secp256k1_num_cmp(&sec, &secp256k1_ge_consts->order) < 0);
+    secp256k1_num_clear(&sec);
     secp256k1_num_free(&sec);
     return ret;
 }
@@ -141,6 +148,8 @@ int secp256k1_ecdsa_pubkey_create(unsigned char *pubkey, int *pubkeylen, const u
     secp256k1_num_set_bin(&sec, seckey, 32);
     secp256k1_gej_t pj;
     secp256k1_ecmult_gen(&pj, &sec);
+    secp256k1_num_clear(&sec);
+    secp256k1_num_free(&sec);
     secp256k1_ge_t p;
     secp256k1_ge_set_gej(&p, &pj);
     secp256k1_ecdsa_pubkey_serialize(&p, pubkey, pubkeylen, compressed);
@@ -173,6 +182,8 @@ int secp256k1_ecdsa_privkey_tweak_add(unsigned char *seckey, const unsigned char
     }
     if (ret)
         secp256k1_num_get_bin(seckey, 32, &sec);
+    secp256k1_num_clear(&sec);
+    secp256k1_num_clear(&term);
     secp256k1_num_free(&sec);
     secp256k1_num_free(&term);
     return ret;