Skip to content

signing.c writes the sighash byte one/two positions past MultisigRedeemScriptType.signatures[i].bytes[73] #518

Description

@BitHighlander

Finding

Severity: high · Dimension: proto-bounds · Location: lib/firmware/signing.c:1499

With signatures[i].size == 74 (accepted by pb_decode, proven by the control run), bytes[74] is 2 bytes past the 73-byte array and 1 byte past the whole 76-byte element: for i < 14 it lands exactly on signatures[i+1].size (little-endian low byte), for i == 14 it lands on MultisigRedeemScriptType.has_m. Concrete case: a host sends signatures_count = 2 with signatures[0].size = 74 and signatures[1].size = 0. The loop writes sighash (0x01) into signatures[1].size, so the size == 0 skip no longer fires on the next iteration and the device emits an extra 2-byte witness element built from a slot the host left empty — the witness stack no longer matches what was reviewed or hashed. With size == 73 the same statement writes into the element's trailing pad byte, and tx_serialize_script(size + 1, ...) then reads 74-75 bytes out of a 73-byte array.

Evidence

lib/firmware/signing.c:1493-1502 (segwit multisig witness assembly):
      for (uint32_t i = 0; i < txinput->multisig.signatures_count; i++) {
        if (txinput->multisig.signatures[i].size == 0) {
          continue;
        }
        nwitnesses++;
        txinput->multisig.signatures[i]
            .bytes[txinput->multisig.signatures[i].size] = sighash;
        r += tx_serialize_script(txinput->multisig.signatures[i].size + 1,
                                 txinput->multisig.signatures[i].bytes,
                                 resp.serialized.serialized_tx.bytes + r);

lib/transport/types.pb.h:269-274:
  typedef PB_BYTES_ARRAY_T(73) MultisigRedeemScriptType_signatures_t;
  typedef struct _MultisigRedeemScriptType {
      pb_size_t pubkeys_count;
      HDNodePathType pubkeys[15];
      pb_size_t signatures_count;
      MultisigRedeemScriptType_signatures_t signatures[15];
      bool has_m;
      uint32_t m;
  } MultisigRedeemScriptType;

Measured layout: element stride 76, signatures[0] at struct offset 2346, signatures[1] at 2422, has_m at 3486, &signatures[14].bytes[73] at 3485.
Decode control (see previous finding): signatures[i].size can legitimately be 74 on the wire.

Suggested fix

Same root fix: validate signatures[i].size <= 72 at input-validation time. Independently, stop appending the sighash in place — build the witness element into a local uint8_t[73+1] scratch (copy size bytes, append sighash) instead of writing one past the decoded field.

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 ?.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions