Merge pull request #105 from obsidiansystems/audit-cleanups

Code fixes from audit
obsidiansystems · Nov 26, 2019 · 8fca217 · 8fca217
2 parents d03e108 + 74e9ed2
commit 8fca217
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 14 deletions.
diff --git a/src/apdu_baking.c b/src/apdu_baking.c
@@ -19,7 +19,7 @@ static bool reset_ok(void);
 size_t handle_apdu_reset(__attribute__((unused)) uint8_t instruction) {
     uint8_t *dataBuffer = G_io_apdu_buffer + OFFSET_CDATA;
     uint32_t dataLength = G_io_apdu_buffer[OFFSET_LC];
-    if (dataLength != sizeof(int)) {
+    if (dataLength != sizeof(level_t)) {
         THROW(EXC_WRONG_LENGTH_FOR_INS);
     }
     level_t const lvl = READ_UNALIGNED_BIG_ENDIAN(level_t, dataBuffer);

diff --git a/src/base58.c b/src/base58.c
@@ -31,7 +31,7 @@ bool b58enc(/* out */ char *b58, /* in/out */ size_t *b58sz, const void *data, s
 
     for (i = zcount, high = size - 1; i < binsz; ++i, high = j)
     {
-        for (carry = bin[i], j = size - 1; (j > high) || carry; --j)
+        for (carry = bin[i], j = size - 1; ((int)j >= 0) && ((j > high) || carry); --j)
         {
             carry += 256 * buf[j];
             buf[j] = carry % 58;

diff --git a/src/keys.c b/src/keys.c
@@ -64,17 +64,23 @@ key_pair_t *generate_key_pair_return_global(
             priv->private_key_data, NULL);
     }
 
-    cx_ecfp_init_private_key(cx_curve, priv->private_key_data, sizeof(priv->private_key_data), &priv->res.private_key);
-    cx_ecfp_generate_pair(cx_curve, &priv->res.public_key, &priv->res.private_key, 1);
-
-    if (cx_curve == CX_CURVE_Ed25519) {
-        cx_edward_compress_point(
-            CX_CURVE_Ed25519,
-            priv->res.public_key.W,
-            priv->res.public_key.W_len);
-        priv->res.public_key.W_len = 33;
+    BEGIN_TRY {
+        TRY {
+            cx_ecfp_init_private_key(cx_curve, priv->private_key_data, sizeof(priv->private_key_data), &priv->res.private_key);
+            cx_ecfp_generate_pair(cx_curve, &priv->res.public_key, &priv->res.private_key, 1);
+
+            if (cx_curve == CX_CURVE_Ed25519) {
+                cx_edward_compress_point(CX_CURVE_Ed25519,
+                                         priv->res.public_key.W,
+                                         priv->res.public_key.W_len);
+                priv->res.public_key.W_len = 33;
+            }
+        } FINALLY {
+            explicit_bzero(priv->private_key_data, sizeof(priv->private_key_data));
+        }
     }
-    memset(priv->private_key_data, 0, sizeof(priv->private_key_data));
+    END_TRY;
+
     return &priv->res;
 }
 
@@ -84,7 +90,7 @@ cx_ecfp_public_key_t const *generate_public_key_return_global(
 ) {
     check_null(bip32_path);
     key_pair_t *const pair = generate_key_pair_return_global(curve, bip32_path);
-    memset(&pair->private_key, 0, sizeof(pair->private_key));
+    explicit_bzero(&pair->private_key, sizeof(pair->private_key));
     return &pair->public_key;
 }
 

diff --git a/src/keys.h b/src/keys.h
@@ -35,7 +35,7 @@ static inline void generate_key_pair(
     check_null(out);
     key_pair_t *const result = generate_key_pair_return_global(derivation_type, bip32_path);
     memcpy(out, result, sizeof(*out));
-    memset(result, 0, sizeof(*result));
+    explicit_bzero(result, sizeof(*result));
 }
 
 // Non-reentrant