App hardening and Zemu test environment updates

app/Makefile.version

@@ -1,3 +1,3 @@
 APPVERSION_M=2
 APPVERSION_N=8
-APPVERSION_P=2
+APPVERSION_P=3

app/rust/src/lib.rs

@@ -102,6 +102,20 @@ pub extern "C" fn sign_sr25519_phase1(
 ) {
     c_zemu_log_stack(b"sign_sr25519\x00".as_ref());
 
+    // Null guards at the FFI boundary. The current in-repo C callers all
+    // pass valid stack buffers, but passing NULL to `from_raw_parts` is UB
+    // regardless of whether the slice is later read. Silent early-return
+    // keeps the `void` signature unchanged; callers that pass NULL get a
+    // no-op and an unchanged signature buffer.
+    if sk_ristretto_expanded_ptr.is_null()
+        || pk_ptr.is_null()
+        || context_ptr.is_null()
+        || msg_ptr.is_null()
+        || sig_ptr.is_null()
+    {
+        return;
+    }
+
     let sk_ristretto_expanded =
         unsafe { from_raw_parts(sk_ristretto_expanded_ptr as *const u8, 64) };
     let pk = unsafe { from_raw_parts(pk_ptr as *const u8, 32) };
@@ -131,6 +145,15 @@ pub extern "C" fn sign_sr25519_phase2(
 ) {
     c_zemu_log_stack(b"sign_sr25519\x00".as_ref());
 
+    if sk_ristretto_expanded_ptr.is_null()
+        || pk_ptr.is_null()
+        || context_ptr.is_null()
+        || msg_ptr.is_null()
+        || sig_ptr.is_null()
+    {
+        return;
+    }
+
     let sk_ristretto_expanded =
         unsafe { from_raw_parts(sk_ristretto_expanded_ptr as *const u8, 64) };
     let pk = unsafe { from_raw_parts(pk_ptr as *const u8, 32) };
@@ -158,6 +181,9 @@ pub extern "C" fn sign_sr25519_phase2(
 
 #[no_mangle]
 pub extern "C" fn expanded_sr25519_sk(sk_ed25519_ptr: *mut u8, sk_ed25519_expanded_ptr: *mut u8) {
+    if sk_ed25519_ptr.is_null() || sk_ed25519_expanded_ptr.is_null() {
+        return;
+    }
     let sk_ed25519 = unsafe { from_raw_parts_mut(sk_ed25519_ptr as *mut u8, 32) };
     let sk_ed25519_expanded = unsafe { from_raw_parts_mut(sk_ed25519_expanded_ptr as *mut u8, 64) };
     let secret: MiniSecretKey = MiniSecretKey::from_bytes(&sk_ed25519[..]).unwrap_handler();
@@ -166,6 +192,9 @@ pub extern "C" fn expanded_sr25519_sk(sk_ed25519_ptr: *mut u8, sk_ed25519_expand
 
 #[no_mangle]
 pub extern "C" fn get_sr25519_sk(sk_ed25519_expanded_ptr: *mut u8) {
+    if sk_ed25519_expanded_ptr.is_null() {
+        return;
+    }
     let sk_ed25519_expanded = unsafe { from_raw_parts_mut(sk_ed25519_expanded_ptr as *mut u8, 64) };
     let secret: SecretKey =
         SecretKey::from_ed25519_bytes(&sk_ed25519_expanded[..]).unwrap_handler();

app/src/apdu_handler.c

@@ -37,10 +37,19 @@
 
 static bool tx_initialized = false;
 
+// Storage for the review-pending re-entrancy lock declared in actions.h.
+// Gates handleApdu so no new APDU can mutate tx / hdPath / G_io_apdu_buffer
+// state while a user review is on screen (BLE transport in particular has
+// no equivalent SDK-level guard).
+volatile bool g_review_pending = false;
+
 static const char *msg_error1 = "Expert Mode";
 static const char *msg_error2 = "Required";
 
 void extractHDPath(uint32_t rx, uint32_t offset) {
+    if (rx < offset) {
+        THROW(APDU_CODE_WRONG_LENGTH);
+    }
     MEMZERO(hdPath, sizeof(hdPath));
     if ((rx - offset) == sizeof(uint32_t) * HDPATH_LEN_ADR0008) {
         hdPathLen = HDPATH_LEN_ADR0008;
@@ -64,14 +73,27 @@ void extractHDPath(uint32_t rx, uint32_t offset) {
         THROW(APDU_CODE_DATA_INVALID);
     }
 
-    if (hdPathLen == HDPATH_LEN_ADR0008 && hdPath[2] < 0x80000000) {
+    // Account index (BIP44 component 2) must be hardened for both the
+    // 3-element ADR-0008 and the 5-element default shapes. Leaving the
+    // account index unhardened would let a host request signatures under
+    // a non-BIP44 account, providing a silent address-enumeration
+    // primitive when paired with INS_GET_ADDR_* with no confirmation.
+    if (hdPath[2] < 0x80000000) {
         THROW(APDU_CODE_DATA_INVALID);
     }
 }
 
 void extract_eth_path(uint32_t rx, uint32_t offset) {
     tx_initialized = false;
 
+    // Reject before reading the path-length byte: rx == offset is
+    // reachable with a truncated APDU (rx == APDU_MIN_LENGTH ==
+    // OFFSET_DATA), which would otherwise read past the received data
+    // and underflow the length check below.
+    if (rx <= offset) {
+        THROW(APDU_CODE_WRONG_LENGTH);
+    }
+
     uint32_t path_len = *(G_io_apdu_buffer + offset);
 
     if (path_len > MAX_BIP32_PATH || path_len < 1) {
@@ -97,6 +119,14 @@ void extract_eth_path(uint32_t rx, uint32_t offset) {
         THROW(APDU_CODE_DATA_INVALID);
     }
 
+    // Require the account index to be hardened when the path reaches
+    // component 2, matching the invariant enforced on the consensus side
+    // (extractHDPath above) and closing the unhardened-account
+    // enumeration primitive on EVM-side GET_ADDR.
+    if (path_len >= 3 && hdPath[2] < 0x80000000) {
+        THROW(APDU_CODE_DATA_INVALID);
+    }
+
     // set the hdPath len
     hdPathLen = path_len;
 }
@@ -449,6 +479,14 @@ void handleApdu(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
 
     BEGIN_TRY {
         TRY {
+            // Reject every incoming APDU while a user review is pending.
+            // Every terminal callback in actions.c clears the flag; a
+            // concurrent APDU landing during the review window throws here
+            // without touching shared state.
+            if (review_is_pending()) {
+                THROW(APDU_CODE_COMMAND_NOT_ALLOWED);
+            }
+
             uint8_t cla = G_io_apdu_buffer[OFFSET_CLA];
             if ((cla != CLA) && (cla != CLA_ETH)) {
                 THROW(APDU_CODE_CLA_NOT_SUPPORTED);
@@ -524,8 +562,18 @@ void handleApdu(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
                 default:
                     THROW(APDU_CODE_INS_NOT_SUPPORTED);
             }
+
+            // If the dispatched handler entered an async user review,
+            // arm the re-entrancy lock. Cleared by each terminal
+            // callback in actions.c.
+            if ((*flags & IO_ASYNCH_REPLY) != 0) {
+                review_mark_pending();
+            }
+        }
+        CATCH(EXCEPTION_IO_RESET) {
+            review_clear_pending();
+            THROW(EXCEPTION_IO_RESET);
         }
-        CATCH(EXCEPTION_IO_RESET) { THROW(EXCEPTION_IO_RESET); }
         CATCH_OTHER(e) {
             switch (e & 0xF000) {
                 case 0x6000:
@@ -536,6 +584,13 @@ void handleApdu(volatile uint32_t *flags, volatile uint32_t *tx, uint32_t rx) {
                     sw = 0x6800 | (e & 0x7FF);
                     break;
             }
+            // Clear the review lock on every error path except the
+            // one that rejected an incoming APDU while a review was
+            // pending — in that case the legitimate in-flight review
+            // must keep its lock.
+            if (e != APDU_CODE_COMMAND_NOT_ALLOWED) {
+                review_clear_pending();
+            }
             G_io_apdu_buffer[*tx] = sw >> 8;
             G_io_apdu_buffer[*tx + 1] = sw;
             *tx += 2;

app/src/common/actions.c

@@ -31,6 +31,8 @@
 uint16_t action_addrResponseLen;
 
 void app_sign_ed25519() {
+    review_clear_pending();
+
     uint8_t *signature = G_io_apdu_buffer;
     uint16_t replyLen = 0;
 
@@ -49,6 +51,8 @@ void app_sign_ed25519() {
 }
 
 void app_sign_secp256k1() {
+    review_clear_pending();
+
     uint8_t *signature = G_io_apdu_buffer;
     uint16_t replyLen = 0;
 
@@ -67,6 +71,8 @@ void app_sign_secp256k1() {
 }
 
 void app_sign_sr25519() {
+    review_clear_pending();
+
     uint8_t *signature = G_io_apdu_buffer;
     uint8_t messageDigest[CX_SHA512_SIZE] = {0};
     size_t ctx_len = 0;
@@ -87,6 +93,7 @@ void app_sign_sr25519() {
 }
 
 void app_reject() {
+    review_clear_pending();
     MEMZERO(G_io_apdu_buffer, IO_APDU_BUFFER_SIZE);
     set_code(G_io_apdu_buffer, 0, APDU_CODE_COMMAND_NOT_ALLOWED);
     io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, 2);
@@ -109,6 +116,8 @@ zxerr_t app_fill_address(address_kind_e kind) {
 }
 
 void app_sign_eth() {
+    review_clear_pending();
+
     const uint8_t *message = tx_get_buffer();
     const uint16_t messageLength = tx_get_buffer_length();
     uint16_t replyLen = 0;
@@ -126,6 +135,7 @@ void app_sign_eth() {
 }
 
 void app_reply_address() {
+    review_clear_pending();
     if (action_addrResponseLen == 0) {
         THROW(APDU_CODE_DATA_INVALID);
     }
@@ -134,6 +144,7 @@ void app_reply_address() {
 }
 
 void app_reply_error() {
+    review_clear_pending();
     set_code(G_io_apdu_buffer, 0, APDU_CODE_DATA_INVALID);
     io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, 2);
 }

app/src/common/actions.h

@@ -15,13 +15,20 @@
  ********************************************************************************/
 #pragma once
 
+#include <stdbool.h>
 #include <stdint.h>
 
 #include "coin.h"
 #include "zxerror.h"
 
 extern uint16_t action_addrResponseLen;
 
+extern volatile bool g_review_pending;
+
+static inline bool review_is_pending(void) { return g_review_pending; }
+static inline void review_mark_pending(void) { g_review_pending = true; }
+static inline void review_clear_pending(void) { g_review_pending = false; }
+
 void app_sign_ed25519();
 void app_sign_secp256k1();
 void app_sign_sr25519();

app/src/common/tx.c

@@ -99,7 +99,7 @@ zxerr_t tx_getItem(int8_t displayIdx, char *outKey, uint16_t outKeyLen, char *ou
 
     CHECK_ZXERR(tx_getNumItems(&numItems))
 
-    if (displayIdx < 0 || displayIdx > numItems) {
+    if (displayIdx < 0 || displayIdx >= numItems) {
         return zxerr_no_data;
     }
 

app/src/consumer/cbor_helper.h

@@ -25,6 +25,7 @@ __Z_INLINE parser_error_t parser_mapCborError(CborError err) {
         case CborErrorUnexpectedEOF:
             return parser_cbor_unexpected_EOF;
         case CborErrorMapNotSorted:
+        case CborErrorMapKeysNotUnique:
             return parser_cbor_not_canonical;
         case CborNoError:
             return parser_ok;

app/src/consumer/coin_consumer.h

@@ -74,6 +74,8 @@ extern "C" {
 #define COIN_RATE_DECIMAL_PLACES 5
 
 #define MAX_RATES 10
+#define MAX_BOUNDS 16
+#define MAX_TOKENS 16
 #define MAX_CONTEXT_SIZE 64
 #define MAX_ENTITY_NODES 16
 

app/src/consumer/parser_consumer.c

@@ -365,7 +365,10 @@ parser_error_t parser_printInnerField(ui_field_t *ui_field) {
 
         if (elements_print == 1) {
             cbor_value_advance(&current);
-            snprintf(val + offset, SCREEN_SIZE, "%s", ":");
+            // Bound the colon-separator write against remaining buffer space
+            // (val[SCREEN_SIZE]); SCREEN_SIZE alone would let the write spill
+            // past the end of the stack buffer once offset approached SCREEN_SIZE.
+            snprintf(val + offset, SCREEN_SIZE - offset, "%s", ":");
             offset += 1;
         }
     }
@@ -1447,8 +1450,8 @@ __Z_INLINE parser_error_t parser_printStakingAmendCommissionSchedule(const parse
     }
 
     uint8_t dynDisplayIdx = displayIdx - 1;
-    if (dynDisplayIdx < (int)(parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.rates_length * 2 +
-                              parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.bounds_length * 3)) {
+    if (dynDisplayIdx < (int)((parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.rates_length * 2) +
+                              (parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.bounds_length * 3))) {
         if (dynDisplayIdx / 2 < (int)parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.rates_length) {
             const int8_t index = dynDisplayIdx / 2;
             commissionRateStep_t rate;
@@ -1494,8 +1497,8 @@ __Z_INLINE parser_error_t parser_printStakingAmendCommissionSchedule(const parse
         }
     }
 
-    uint8_t lastDisplayIdx = dynDisplayIdx - parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.rates_length * 2 -
-                             parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.bounds_length * 3;
+    uint8_t lastDisplayIdx = dynDisplayIdx - (parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.rates_length * 2) -
+                             (parser_tx_obj.oasis.tx.body.stakingAmendCommissionSchedule.bounds_length * 3);
     switch (lastDisplayIdx) {
         case 0: {
             snprintf(outKey, outKeyLen, "Fee");