Host-controlled multisig signatures overflow TxInputType.script_sig.bytes[1650] during signing

[BitHighlander]

Finding

Severity: high · Dimension: proto-bounds · Location: lib/firmware/transaction.c:723

A compromised host drives a normal SignTx the user approves (it only sees amounts/fee/address; nothing about multisig blob sizes). For the input being signed it sends a TxInputType with script_type=SPENDMULTISIG (or SPENDP2SHWITNESS/SPENDWITNESS), multisig.pubkeys_count=15 (one entry being the device's own HDNodePathType, obtainable via GetPublicKey so cryptoMultisigPubkeyIndex succeeds at signing.c:1345), multisig.signatures_count=15, and every signatures[i] carrying 74 bytes. signing_sign_hash overwrites one slot with the device's own DER signature (<=72 bytes) and then calls serialize_script_multisig. Length produced = 1 (OP_0) + 14*(op_push(75)=1 + 74 + 1) + (1 + 72 + 1) + op_push(513)=3 + 513 = 1655 bytes written into script_sig.bytes[1650]. Bytes 1650..1654 land on struct offsets 1730..1734, i.e. TxInputType.has_sequence and the low three bytes of TxInputType.sequence. Both signing paths reach it: signing.c:1837 memcpy's the wire input into the static input then signing.c:1386 signs it (non-segwit), and signing.c:2052 calls signing_sign_segwit_input(&tx->inputs[0]) which sits inside the 13KB decode_buffer. Concrete wrong outcome: an out-of-bounds write past a declared array in the signing path, and because nSequence is clobbered AFTER tx_hash_final() computed the sighash but BEFORE tx_serialize_input() emits the input, the device returns a signature over one nSequence while serializing a different one, plus a script_sig.size of 1655 that later feeds tx_script_hash()/tx_serialize_input() as a length for a 1650-byte array.

Evidence

lib/firmware/transaction.c:710-736 (serialize_script_multisig) has NO total-length bound on `out`:
  uint32_t r = 0;
  if (!coin->decred) { out[r] = 0x00; r++; }
  for (uint32_t i = 0; i < multisig->signatures_count; i++) {
    if (multisig->signatures[i].size == 0) { continue; }
    r += op_push(multisig->signatures[i].size + 1, out + r);
    memcpy(out + r, multisig->signatures[i].bytes, multisig->signatures[i].size);
    r += multisig->signatures[i].size;
    out[r] = sighash;
    r++;
  }
  uint32_t script_len = compile_script_multisig(coin, multisig, 0);
  ...
  r += op_push(script_len, out + r);
  r += compile_script_multisig(coin, multisig, out + r);

The only caller supplies a fixed 1650-byte buffer, lib/firmware/signing.c:1356-1357:
    txinput->script_sig.size = serialize_script_multisig(
        coin, &(txinput->multisig), sighash, txinput->script_sig.bytes);

lib/transport/types.options:
  TxInputType.script_sig		max_size:1650
  MultisigRedeemScriptType.pubkeys	max_count:15
  MultisigRedeemScriptType.signatures	max_count:15 max_size:73

The 1650 cap only holds if each signature is <= 73 bytes, but nanopb accepts 74. lib/transport/pb_decode.c:1311-1323:
  alloc_size = PB_BYTES_ARRAY_T_ALLOCSIZE(size);   /* = size + 2 */
  ...
    if (alloc_size > field->data_size) PB_RETURN_ERROR(stream, "bytes overflow");
and PB_REPEATED_STATIC sets data_size = pb_membersize(st, m[0]) = sizeof(MultisigRedeemScriptType_signatures_t) = 76 (uint16 size + uint8 bytes[73] + 1 pad), so the accepted maximum is 76-2 = 74.

RUNNING CONTROL against this tree's pb_decode.c + types.pb.c (decoding field 2 of MultisigRedeemScriptType):
  len=72 -> decode OK, stored size=72
  len=73 -> decode OK, stored size=73
  len=74 -> decode OK, stored size=74
  len=75 -> decode FAIL
  len=76 -> decode FAIL
(75/76 failing proves the check can fail, so the 74 result is meaningful.)

Struct layout, measured from this tree's headers:
  off script_sig.bytes=80, end of script_sig.bytes=1730, off has_sequence=1730, off sequence=1732

Suggested fix

Reject the message before use: in signing_validate_input() (lib/firmware/signing.c:826), loop over txinput->multisig.signatures_count and fail with Failure_SyntaxError if any signatures[i].size > 72 (max DER ECDSA) — never rely on the .options max_size as the runtime bound. Additionally give serialize_script_multisig() an explicit out_len parameter and have it return 0 when r would exceed it.

Verification

(not independently verified)

---

Found by an adversarial audit of release/7.15 (628b09257). Each finding was independently re-checked by a separate reviewer instructed to refute it by default; this one survived at confidence ?.