fix audit findings

Zondax · Feb 4, 2025 · d10792d · d10792d
1 parent cbbcf79
commit d10792d
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 6 deletions.
diff --git a/src/random_oracle_input.c b/src/random_oracle_input.c
@@ -82,11 +82,15 @@ void roinput_add_bytes(ROInput *input, const uint8_t *bytes, size_t len)
     input->bits_len += 8 * len;
 }
 
-void roinput_add_bytes_le(ROInput *input, const uint8_t *bytes, size_t len)
+int roinput_add_bytes_le(ROInput *input, const uint8_t *bytes, size_t len)
 {
+    if (input == NULL || bytes == NULL) {
+        return -1;
+    }
+
     size_t remaining = (int)input->bits_capacity * 8 - (int)input->bits_len;
     if (remaining < 8 * len) {
-        return;
+        return -1;
     }
     // LSB bits
     size_t k = input->bits_len;
@@ -98,6 +102,7 @@ void roinput_add_bytes_le(ROInput *input, const uint8_t *bytes, size_t len)
         }
     }
     input->bits_len += 8 * len;
+    return 0;
 }
 
 void roinput_add_uint32(ROInput *input, const uint32_t x)

diff --git a/src/random_oracle_input.h b/src/random_oracle_input.h
@@ -25,7 +25,7 @@ void roinput_add_field(ROInput *input, const Field a);
 void roinput_add_scalar(ROInput *input, const Scalar a);
 void roinput_add_bit(ROInput *input, const bool b);
 void roinput_add_bytes(ROInput *input, const uint8_t *bytes, size_t len);
-void roinput_add_bytes_le(ROInput *input, const uint8_t *bytes, size_t len);
+int roinput_add_bytes_le(ROInput *input, const uint8_t *bytes, size_t len);
 void roinput_add_uint32(ROInput *input, const uint32_t x);
 void roinput_add_uint64(ROInput *input, const uint64_t x);
 int roinput_derive_message(uint8_t *out, const size_t len, const Keypair *kp, const ROInput *msg, const uint8_t network_id);

diff --git a/src/sign_msg.c b/src/sign_msg.c
@@ -43,14 +43,21 @@ void sign_message(uint8_t *dataBuffer, uint8_t dataLength)
         }
     }
 
-    generate_keypair(&kp, account);
+    if (roinput_add_bytes_le(&roinput, dataBuffer + MSG_OFFSET, dataLength - (ACCOUNT_LENGTH + NETWORK_LENGTH)) < 0) {
+        THROW(INVALID_PARAMETER);
+    }
 
-    roinput_add_bytes_le(&roinput, dataBuffer + MSG_OFFSET, dataLength - (ACCOUNT_LENGTH + NETWORK_LENGTH));
+    generate_keypair(&kp, account);
 
     if (!sign(&sig, &kp, &roinput, network)) {
+        // Clear secret from stack
+        memset(&kp, 0, sizeof(kp));
         THROW(INVALID_PARAMETER);
     }
 
+    // Clear secret from stack
+    memset(&kp, 0, sizeof(kp));
+
     memmove(G_io_apdu_buffer, &sig, sizeof(sig));
 
     sendResponse(sizeof(sig), true);

diff --git a/src/sign_msg_bagl.c b/src/sign_msg_bagl.c
@@ -96,6 +96,10 @@ UX_FLOW(ux_sign_msg_flow_testnet,
 
 void ui_sign_msg(uint8_t *dataBuffer, uint8_t dataLength)
 {
+    if (dataBuffer == NULL) {
+        THROW(INVALID_PARAMETER);
+    }
+
     _msgData.dataBufLength = dataLength;
     memcpy(_msgData.msgDataBuf, (char *) dataBuffer, _msgData.dataBufLength);
 

diff --git a/src/sign_msg_nbgl.c b/src/sign_msg_nbgl.c
@@ -62,6 +62,10 @@ static void prepare_msg_context(void) {
 
 void ui_sign_msg(uint8_t *dataBuffer, uint8_t dataLength)
 {
+    if (dataBuffer == NULL) {
+        THROW(INVALID_PARAMETER);
+    }
+
     _msgData.dataBufLength = dataLength;
     memcpy(_msgData.msgDataBuf, (char *) dataBuffer, _msgData.dataBufLength);
 

diff --git a/src/utils.c b/src/utils.c
@@ -32,6 +32,9 @@ int b58_encode(const unsigned char *in, unsigned char length,
         // Input buffer too big
         return -1;
     }
+    if (length > (sizeof(buffer) / 2)) {
+        return -1;
+    }
     memcpy(tmp, in, length);
     while ((zeroCount < length) && (tmp[zeroCount] == 0)) {
         ++zeroCount;
@@ -170,7 +173,7 @@ uint64_t read_uint64_be(const uint8_t *buffer)
 
 char *amount_to_string(char *buf, const size_t len, uint64_t amount)
 {
-    // COIN = 1.000 000 000;
+    // COIN is 1.000 000 000;
     size_t mantissa_len = 1;
     for (uint64_t value = amount, _len = 9; value && _len > 0; value /= 10, _len--) {
         if (value % 10 != 0) {