fix: harden PSBT signing against integer UB on malformed input

Author: N3ur0sis

src/handler/lib/check_merkle_tree_sorted.c

@@ -24,10 +24,12 @@ int call_check_merkle_tree_sorted_with_callback(dispatcher_context_t *dispatcher
 
     for (size_t cur_el_idx = 0; cur_el_idx < size; cur_el_idx++) {
         uint8_t cur_el[MAX_CHECK_MERKLE_TREE_SORTED_PREIMAGE_SIZE];
+        // tree size and leaf index are bounded well below 2^32 in practice;
+        // cast to the callee's uint32_t parameters explicitly.
         int cur_el_len = call_get_merkle_leaf_element(dispatcher_context,
                                                       root,
-                                                      size,
-                                                      cur_el_idx,
+                                                      (uint32_t) size,
+                                                      (uint32_t) cur_el_idx,
                                                       cur_el,
                                                       sizeof(cur_el));
 

src/handler/lib/policy.c

@@ -1956,7 +1956,7 @@ static int check_older_node_cb(const policy_node_t *node, void *callback_state)
     (void) callback_state;
     if (node->type == TOKEN_OLDER) {
         const policy_node_with_uint32_t *older = (const policy_node_with_uint32_t *) node;
-        uint32_t n = older->n & ~SEQUENCE_LOCKTIME_TYPE_FLAG;
+        uint32_t n = older->n & ~(uint32_t) SEQUENCE_LOCKTIME_TYPE_FLAG;
         if (n < 1 || n > 65535) {
             return -1;
         }

src/handler/sign_psbt.h

@@ -34,7 +34,7 @@ typedef struct {
                                 // matched with the current key expression in the signing flow
 
     bool is_change;
-    int address_index;
+    uint32_t address_index;
 
     // For an output, its scriptPubKey
     // for an input, the prevout's scriptPubKey (either from the non-witness-utxo, or from the

src/handler/sign_psbt/musig_signing.c

@@ -368,12 +368,14 @@ bool __attribute__((noinline)) sign_sighash_musig_and_yield(dispatcher_context_t
         memcpy(musig_my_psbt_id + 33 + 33, keyexpr_info->tapleaf_hash, 32);
     }
     musig_pubnonce_t my_pubnonce;
-    if (sizeof(musig_pubnonce_t) != call_get_merkleized_map_value(dc,
-                                                                  &input->in_out.map,
-                                                                  musig_my_psbt_id_key,
-                                                                  1 + psbt_id_len,
-                                                                  my_pubnonce.raw,
-                                                                  sizeof(musig_pubnonce_t))) {
+    // call_get_merkleized_map_value returns int (negative on error); cast the
+    // unsigned sizeof so the comparison doesn't trip UBSan's sign-change check.
+    if ((int) sizeof(musig_pubnonce_t) != call_get_merkleized_map_value(dc,
+                                                                        &input->in_out.map,
+                                                                        musig_my_psbt_id_key,
+                                                                        1 + psbt_id_len,
+                                                                        my_pubnonce.raw,
+                                                                        sizeof(musig_pubnonce_t))) {
         PRINTF("Missing or erroneous pubnonce in PSBT\n");
         SEND_SW(dc, SW_INCORRECT_DATA);
         return false;

src/handler/sign_psbt/preprocess_inputs.c

@@ -178,6 +178,12 @@ bool __attribute__((noinline)) preprocess_inputs(
                 return false;
             }
 
+            // sanity check before accumulating, to avoid overflowing the total
+            if (input.prevout_amount > BITCOIN_TOTAL_SUPPLY) {
+                PRINTF("Input amount exceeds Bitcoin total supply!\n");
+                SEND_SW(dc, SW_INCORRECT_DATA);
+                return false;
+            }
             st->inputs_total_amount += input.prevout_amount;
         }
 
@@ -212,6 +218,12 @@ bool __attribute__((noinline)) preprocess_inputs(
                 }
             } else {
                 // we extract the scriptPubKey and prevout amount from the witness utxo
+                // sanity check before accumulating, to avoid overflowing the total
+                if (wit_utxo_prevout_amount > BITCOIN_TOTAL_SUPPLY) {
+                    PRINTF("Input amount exceeds Bitcoin total supply!\n");
+                    SEND_SW(dc, SW_INCORRECT_DATA);
+                    return false;
+                }
                 st->inputs_total_amount += wit_utxo_prevout_amount;
 
                 input.prevout_amount = wit_utxo_prevout_amount;
@@ -220,13 +232,6 @@ bool __attribute__((noinline)) preprocess_inputs(
             }
         }
 
-        if (input.prevout_amount > BITCOIN_TOTAL_SUPPLY) {
-            // sanity check to avoid overflows in amounts
-            PRINTF("Input amount exceed Bitcoin total supply!\n");
-            SEND_SW(dc, SW_INCORRECT_DATA);
-            return false;
-        }
-
         // check if the input is internal; if not, continue
 
         int is_internal = is_in_out_internal(dc, st, sign_psbt_cache, &input.in_out, true);