Different fixes

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 All notable changes to this project will be documented in this file.
 
+
+## 2.2.1 - 2026-06-11
+
+- Different security and bug fixes
+
 ## 2.2.0 - 2026-05-11
 
 - Different security fixes

Makefile

@@ -29,7 +29,7 @@ APPNAME = "Monero"
 # Application version
 APPVERSION_M = 2
 APPVERSION_N = 2
-APPVERSION_P = 0
+APPVERSION_P = 1
 APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"
 
 SPECVERSION = "1.0"
@@ -79,9 +79,11 @@ DEFINES   += HAVE_LEGACY_PID
 DEFINES   += U2F_PROXY_MAGIC=\"MOON\"
 DEFINES   += HAVE_IO_U2F HAVE_U2F
 
-# Enabling debug PRINTF
-ifneq ($(DEBUG),0)
-  # Debug options
+# The gate is opt-in: a bare `make` (DEBUG unset) or `make DEBUG=0`
+# builds the secure configuration, and only an explicit `make DEBUG=1`
+# enables them (used by functional tests)
+ifeq ($(DEBUG),1)
+  # Enabling debug PRINTF
   DEFINES += DEBUG_HWDEVICE
   DEFINES += BYPASS_COMMITMENT_FOR_TESTS
   DEFINES += IODUMMYCRYPT  # or IONOCRYPT

src/monero_api.h

@@ -194,7 +194,13 @@ int monero_get_subaddress(unsigned char *C, unsigned char *D, unsigned char *ind
                           size_t D_len, size_t index_len);
 int monero_get_subaddress_secret_key(unsigned char *sub_s, unsigned char *s, unsigned char *index,
                                      size_t sub_s_len, size_t s_len, size_t index_len);
-int monero_check_change_address(const unsigned char *Aout, const unsigned char *Bout);
+// On a successful match, *out_major (if non-NULL) receives the change account
+// index (0 for the primary address). Change always lands on the account root
+// (minor 0), so only the account is reported.
+int monero_check_change_address(const unsigned char *Aout, const unsigned char *Bout,
+                                unsigned int *out_major);
+// Build the change-review title ("Change" or "Change account <major>").
+void monero_format_change_title(char *str, size_t str_len, unsigned int major);
 
 /* ----------------------------------------------------------------------- */
 /* ---                              CRYPTO                            ---- */
@@ -257,6 +263,38 @@ static inline int monero_sha256_outkeys_final(unsigned char *out) {
     return monero_hash_final((cx_hash_t *)&G_monero_vstate.sha256_out_keys, out);
 }
 
+static inline void monero_sha256_out_eph_init(void) {
+    monero_hash_init_sha256((cx_hash_t *)&G_monero_vstate.sha256_out_eph);
+}
+
+static inline int monero_sha256_out_eph_update(const unsigned char *buf, size_t len) {
+    return monero_hash_update((cx_hash_t *)&G_monero_vstate.sha256_out_eph, buf, len);
+}
+
+static inline int monero_sha256_out_eph_final(unsigned char *out) {
+    return monero_hash_final((cx_hash_t *)&G_monero_vstate.sha256_out_eph, out);
+}
+
+static inline void monero_sha256_addk_init(void) {
+    monero_hash_init_sha256((cx_hash_t *)&G_monero_vstate.sha256_addk);
+}
+
+static inline int monero_sha256_addk_update(const unsigned char *buf, size_t len) {
+    return monero_hash_update((cx_hash_t *)&G_monero_vstate.sha256_addk, buf, len);
+}
+
+static inline int monero_sha256_addk_final(unsigned char *out) {
+    return monero_hash_final((cx_hash_t *)&G_monero_vstate.sha256_addk, out);
+}
+
+/* Reset the tx-prefix output-key parser before streaming a new prefix. */
+void monero_prefix_outkeys_reset(void);
+
+/* Feed the next tx-prefix chunk (vin/vout/extra, as streamed over
+ * INS_PREFIX_HASH P2) to the output-key check. Returns an error if the signed
+ * outputs don't match the ones derived in INS_GEN_TXOUT_KEYS. */
+int monero_prefix_outkeys_parse(const unsigned char *buf, size_t len);
+
 /*
  *  check 1<s<N, else throw
  */

src/monero_clsag.c

@@ -191,6 +191,13 @@ int monero_apdu_clsag_sign() {
     unsigned char mu_C[32];
     int err = 0;
 
+    // Never sign a real transaction the user did not approve on screen.
+    if (G_monero_vstate.tx_sig_mode == TRANSACTION_CREATE_REAL &&
+        !G_monero_vstate.user_approved_tx) {
+        err = SW_SECURITY_USER_NOT_APPROVED;
+        goto end;
+    }
+
     if (G_monero_vstate.tx_sig_mode == TRANSACTION_CREATE_FAKE) {
         monero_io_fetch(a, 32);
         monero_io_fetch(p, 32);

src/monero_crypto.c

@@ -105,14 +105,19 @@ int monero_aes_derive(cx_aes_key_t *sk, unsigned char *seed32, unsigned char *a,
 }
 
 /* ----------------------------------------------------------------------- */
-/* --- assert: max_len>0                                               --- */
+/* --- max_len==0 and overflow are handled internally                  --- */
 /* ----------------------------------------------------------------------- */
 unsigned int monero_encode_varint(unsigned char *varint, unsigned int max_len, uint64_t value,
                                   unsigned int *out_len) {
     if (!varint || !out_len) {
         PRINTF("Buffer Error: %s:%d \n", __LINE__);
         return SW_WRONG_DATA;
     }
+    // Reject max_len==0 up front: otherwise (max_len - 1) underflows and the
+    // bound check below never fires, allowing an out-of-bounds write.
+    if (max_len == 0) {
+        return SW_WRONG_DATA_RANGE;
+    }
 
     *out_len = 0;
     while (value >= 0x80) {
@@ -129,30 +134,39 @@ unsigned int monero_encode_varint(unsigned char *varint, unsigned int max_len, u
 }
 
 /* ----------------------------------------------------------------------- */
-/* --- assert: max_len>0                                               --- */
+/* --- length-driven varint decoder: never reads past max_len bytes,    --- */
+/* --- rejects max_len==0, and caps at a canonical 10-byte uint64 to     --- */
+/* --- avoid a (len*7) shift past 63 bits                                --- */
 /* ----------------------------------------------------------------------- */
 unsigned int monero_decode_varint(const unsigned char *varint, size_t max_len, uint64_t *value,
                                   unsigned int *out_len) {
-    uint64_t v;
-    size_t len;
-    v = 0;
-    len = 0;
-    if (!varint || !out_len) {
+    if (!varint || !value || !out_len) {
         PRINTF("Buffer Error: %s:%d \n", __LINE__);
         return SW_WRONG_DATA;
     }
-    while ((varint[len]) & 0x80) {
-        if (len == (max_len - 1)) {
+    if (max_len == 0) {
+        return SW_WRONG_DATA_RANGE;
+    }
+    uint64_t v   = 0;
+    size_t   len = 0;
+    while (len < max_len) {
+        unsigned char byte = varint[len];
+        // A uint64 varint is at most 10 bytes; the 10th byte (index 9) may only
+        // carry bit 63, i.e. its value must be 0 or 1. Reject anything longer or
+        // non-canonical so the shift below never exceeds 63.
+        if (len >= 10 || (len == 9 && (byte & 0xFE) != 0)) {
             return SW_WRONG_DATA_RANGE;
         }
-        v = v + ((uint64_t)((varint[len]) & 0x7f) << (len * 7));
+        v |= ((uint64_t)(byte & 0x7f)) << (len * 7);
         len++;
+        if ((byte & 0x80) == 0) {
+            *value   = v;
+            *out_len = (unsigned int) len;
+            return 0;
+        }
     }
-
-    v = v + ((uint64_t)((varint[len]) & 0x7f) << (len * 7));
-    *value = v;
-    *out_len = len + 1;
-    return 0;
+    // Ran out of bytes before the terminator: truncated / unterminated varint.
+    return SW_WRONG_DATA_RANGE;
 }
 
 /* ----------------------------------------------------------------------- */

src/monero_io.c

@@ -343,6 +343,7 @@ int monero_io_fetch_decrypt_key(unsigned char* buffer, size_t buffer_size) {
             if (error) {
                 return error;
             }
+            G_monero_vstate.io_offset += 32;  // consume the HMAC, as the view branch does
         }
         memcpy(buffer, G_monero_vstate.b, 32);
         return 0;
@@ -354,15 +355,21 @@ int monero_io_fetch_decrypt_key(unsigned char* buffer, size_t buffer_size) {
 }
 
 int monero_io_fetch_varint(uint64_t* out_v64) {
-    if (!out_v64) {
-        return SW_WRONG_DATA;
+    int available = monero_io_fetch_available();
+    // Refuse to decode when no APDU bytes remain: otherwise max_len would be 0
+    // and the decoder could read past the payload.
+    if (!out_v64 || available <= 0) {
+        return SW_WRONG_DATA_RANGE;
     }
     unsigned int out_len = 0;
-    unsigned int error = monero_decode_varint(
+    unsigned int error   = monero_decode_varint(
         G_monero_vstate.io_buffer + G_monero_vstate.io_offset,
-        MIN(8, G_monero_vstate.io_length - G_monero_vstate.io_offset), out_v64, &out_len);
+        MIN(10, (size_t) available), out_v64, &out_len);
+    if (error) {
+        return error;
+    }
     G_monero_vstate.io_offset += out_len;
-    return error;
+    return 0;
 }
 
 unsigned int monero_io_fetch_u32(void) {
@@ -440,6 +447,10 @@ int monero_io_do(unsigned int io_flags) {
     }
     // else send data now
     else {
+        // A reply is going out, so the command it answers no longer owes one.
+        // (Covers both synchronous replies and the deferred IO_RETURN_AFTER_TX
+        // sent from a confirmation callback.)
+        G_monero_vstate.io_reply_pending = 0;
         G_monero_vstate.io_offset = 0;
         if (G_monero_vstate.io_length > MAX_OUT) {
             return SW_IO_FULL;

src/monero_key.c

@@ -25,13 +25,14 @@
 /* ----------------------------------------------------------------------- */
 /* ---                                                                 --- */
 /* ----------------------------------------------------------------------- */
-static void monero_payment_id_to_str(const unsigned char *payment_id, char *str) {
+static void monero_payment_id_to_str(const unsigned char *payment_id, char *str, size_t str_len) {
+    // 8 bytes -> 16 hex chars + NUL; the last snprintf writes the terminator at
+    // str[16], so the destination must hold at least 17 bytes.
+    if (str_len < 17) {
+        return;
+    }
     for (int i = 0; i < 8; i++) {
-        if (payment_id[i] <= 0xF) {
-            snprintf(str + i * 2, 3, "0%x", payment_id[i]);
-        } else {
-            snprintf(str + i * 2, 3, "%x", payment_id[i]);
-        }
+        snprintf(str + i * 2, str_len - (i * 2), "%02x", payment_id[i]);
     }
 }
 
@@ -73,7 +74,8 @@ int monero_apdu_display_address() {
         G_monero_vstate.disp_addr_mode = DISP_SUB;
     } else {
         if (G_monero_vstate.io_p1 == 1) {
-            monero_payment_id_to_str(payment_id, G_monero_vstate.payment_id);
+            monero_payment_id_to_str(payment_id, G_monero_vstate.payment_id,
+                                     sizeof(G_monero_vstate.payment_id));
             G_monero_vstate.disp_addr_mode = DISP_INTEGRATED;
         } else {
             G_monero_vstate.disp_addr_mode = DISP_MAIN;
@@ -166,7 +168,8 @@ int monero_apdu_get_key() {
                 (char *)G_monero_vstate.io_buffer + G_monero_vstate.io_offset, G_monero_vstate.A,
                 G_monero_vstate.B, 0, NULL);
             if (error) {
-                return 0;
+                // Return a real error SW
+                return error;
             }
             monero_io_inserted(95);
             break;
@@ -780,7 +783,8 @@ int monero_apdu_get_subaddress_secret_key(/*const crypto::secret_key& sec, const
 /* ----------------------------------------------------------------------- */
 /* ---                                                                 --- */
 /* ----------------------------------------------------------------------- */
-int monero_check_change_address(const unsigned char *Aout, const unsigned char *Bout) {
+int monero_check_change_address(const unsigned char *Aout, const unsigned char *Bout,
+                                unsigned int *out_major) {
     unsigned char C[KEY_SIZE];
     unsigned char D[KEY_SIZE];
     unsigned char index[8];
@@ -789,6 +793,9 @@ int monero_check_change_address(const unsigned char *Aout, const unsigned char *
     /* primary address (major=0, minor=0) */
     if ((memcmp(Aout, G_monero_vstate.A, KEY_SIZE) == 0) &&
         (memcmp(Bout, G_monero_vstate.B, KEY_SIZE) == 0)) {
+        if (out_major != NULL) {
+            *out_major = 0;
+        }
         return 0;
     }
 
@@ -814,11 +821,42 @@ int monero_check_change_address(const unsigned char *Aout, const unsigned char *
                 return err;
             }
             if ((memcmp(Aout, C, KEY_SIZE) == 0) && (memcmp(Bout, D, KEY_SIZE) == 0)) {
+                if (out_major != NULL) {
+                    *out_major = M;
+                }
                 return 0;
             }
         }
     }
 
+    /* The wallet sends change to the account root (M, 0) but never asks the
+     * device for it while signing, so the whitelist above is empty on a normal
+     * send and change to a non-primary account gets rejected. Re-derive each
+     * (M, 0) and match against (Aout, Bout). B can't be forged without the spend
+     * key, so substitution is still caught. Bound matches wallet2's default. */
+#define CHANGE_ACCOUNT_LOOKAHEAD 50
+    index[4] = 0;
+    index[5] = 0;
+    index[6] = 0;
+    index[7] = 0;
+    for (unsigned int M = 1; M < CHANGE_ACCOUNT_LOOKAHEAD; M++) {
+        index[0] = (unsigned char)(M & 0xFF);
+        index[1] = (unsigned char)((M >> 8) & 0xFF);
+        index[2] = (unsigned char)((M >> 16) & 0xFF);
+        index[3] = (unsigned char)((M >> 24) & 0xFF);
+        err = monero_get_subaddress(C, D, index, KEY_SIZE, KEY_SIZE, sizeof(index));
+        if (err) {
+            return err;
+        }
+        if ((memcmp(Aout, C, KEY_SIZE) == 0) && (memcmp(Bout, D, KEY_SIZE) == 0)) {
+            if (out_major != NULL) {
+                *out_major = M;
+            }
+            return 0;
+        }
+    }
+#undef CHANGE_ACCOUNT_LOOKAHEAD
+
     return SW_SECURITY_CHANGE_ADDRESS;
 }
 
@@ -874,12 +912,27 @@ int monero_apu_generate_txout_keys(/*size_t tx_version, crypto::secret_key tx_se
 
     // reject spoofed change address before any state is mutated
     if (is_change && (G_monero_vstate.tx_sig_mode == TRANSACTION_CREATE_REAL)) {
-        err = monero_check_change_address(Aout, Bout);
+        err = monero_check_change_address(Aout, Bout, NULL);
         if (err) {
             goto end;
         }
     }
 
+    // Pin every output to one main tx public key. The wallet computes a single
+    // txkey_pub per tx and reuses it for all outputs -- r.G normally, or r.D for
+    // a single subaddress destination. Recording it from the first output and
+    // enforcing it on the rest stops a host from deriving the change under one
+    // key while a different R lands on-chain via a later output (which would burn
+    // the change), without assuming the key is r.G.
+    if (G_monero_vstate.tx_sig_mode == TRANSACTION_CREATE_REAL) {
+        if (G_monero_vstate.tx_output_cnt == 0) {
+            memcpy(G_monero_vstate.tx_main_txkey, txkey_pub, KEY_SIZE);
+        } else if (memcmp(txkey_pub, G_monero_vstate.tx_main_txkey, KEY_SIZE) != 0) {
+            err = SW_WRONG_DATA;
+            goto end;
+        }
+    }
+
     // update outkeys hash control
     if (G_monero_vstate.tx_sig_mode == TRANSACTION_CREATE_REAL) {
         if (G_monero_vstate.io_protocol_version >= 2) {
@@ -966,6 +1019,36 @@ int monero_apu_generate_txout_keys(/*size_t tx_version, crypto::secret_key tx_se
         }
     }
 
+    // Chain each output key (and view tag) into OUT_EPH as we derive it. The
+    // host puts the same out_eph_public_key into the prefix, so INS_PREFIX_HASH
+    // can re-walk the signed outputs and check them against this chain. The
+    // wallet emits outputs in this call order (construct_tx_with_tx_key), so
+    // both sides hash them the same way.
+    if ((G_monero_vstate.tx_sig_mode == TRANSACTION_CREATE_REAL) &&
+        (G_monero_vstate.io_protocol_version >= 2)) {
+        err = monero_sha256_out_eph_update(out_eph_public_key, KEY_SIZE);
+        if (err) {
+            goto end;
+        }
+        if (use_view_tags) {
+            err = monero_sha256_out_eph_update(view_tag, 1);
+            if (err) {
+                goto end;
+            }
+        }
+        /* Also bind the tx pubkeys that go into `extra`: the main R (txkey_pub,
+         * same for every output) and, when present, the per-output additional
+         * key. INS_PREFIX_HASH checks both against the signed prefix. */
+        memcpy(G_monero_vstate.EXTRA_R, txkey_pub, KEY_SIZE);
+        if (need_additional_txkeys) {
+            G_monero_vstate.prefix_addk_expected = 1;
+            err = monero_sha256_addk_update(additional_txkey_pub, KEY_SIZE);
+            if (err) {
+                goto end;
+            }
+        }
+    }
+
     // send all
     monero_io_discard(0);
     monero_io_insert_encrypt(amount_key, KEY_SIZE, TYPE_AMOUNT_KEY);