Skip to content

Commit b91d87b

Browse files
committed
test: an oversized multisig signature must be refused
MultisigRedeemScriptType.signatures is declared max_size:73, so the decoder accepts 73 bytes, but a DER-encoded ECDSA signature is at most 72: 0x30 len, then two 0x02-tagged integers of at most 33 bytes each. The witness serializer appended the sighash byte AT signatures[i].size, so a 73-byte value wrote one past the end of bytes[73] -- onto signatures[i+1].size for i < 14, which can revive a slot the host deliberately left empty and change the witness stack after the user reviewed it, or onto has_m at i == 14. A declared max_size is a DECODER bound and never a runtime one. Registered as Z27 so it is in the CI filter and actually runs.
1 parent 34a1c6c commit b91d87b

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

scripts/generate-test-report.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,6 +2291,15 @@ def _arg_shown(a):
22912291
'notes, reuse an approved action alpha so rk is byte-identical, and the single '
22922292
'RedPallas signature the device emits verifies in BOTH bundles.',
22932293
[]),
2294+
('Z27', 'test_multisig',
2295+
'test_oversized_signature_is_rejected',
2296+
'Oversized multisig signature refused (ON DEVICE)',
2297+
'MultisigRedeemScriptType.signatures is declared max_size:73 but a DER ECDSA signature '
2298+
'is at most 72. The witness serializer appended the sighash byte AT signatures[i].size, '
2299+
'so 73 wrote one past the end of bytes[73] -- onto signatures[i+1].size for i < 14, '
2300+
'which can revive a slot the host left empty and change the witness stack after the '
2301+
'user reviewed it. A declared max_size is a decoder bound, not a runtime one.',
2302+
[]),
22942303
]),
22952304

22962305
('D', 'BIP-85 Child Derivation', '7.14.0',

tests/test_multisig.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,5 +240,49 @@ def test_missing_pubkey(self):
240240
self.assertRaises(CallException, self.client.sign_tx, 'Bitcoin', [inp1, ], [out1, ])
241241

242242

243+
def test_oversized_signature_is_rejected(self):
244+
"""A multisig signature longer than a DER ECDSA signature is refused.
245+
246+
MultisigRedeemScriptType.signatures is declared max_size:73, so the
247+
decoder accepts 73 bytes -- but a DER-encoded ECDSA signature is at
248+
most 72 (0x30 len, then two 0x02-tagged integers of at most 33 bytes).
249+
The witness serializer used to append the sighash byte AT
250+
signatures[i].size, so a 73-byte value wrote one past the end of
251+
bytes[73]: onto signatures[i+1].size for i < 14, which can revive a
252+
slot the host left empty and change the witness stack after the user
253+
reviewed it, or onto has_m at i == 14.
254+
255+
The declared max_size is a decoder bound, never a runtime one. This
256+
asserts the device applies the real one.
257+
"""
258+
self.setup_mnemonic_nopin_nopassphrase()
259+
260+
node = ckd_public.deserialize('xpub661MyMwAqRbcF1zGijBb2K6x9YiJPh58xpcCeLvTxMX6spkY3PcpJ4ABcCyWfskq5DDxM3e6Ez5ePCqG5bnPUXR4wL8TZWyoDaUdiWW7bKy')
261+
262+
multisig = proto_types.MultisigRedeemScriptType(
263+
pubkeys=[proto_types.HDNodePathType(node=node, address_n=[1]),
264+
proto_types.HDNodePathType(node=node, address_n=[2]),
265+
proto_types.HDNodePathType(node=node, address_n=[3])],
266+
# 73 bytes: one more than any real DER signature,
267+
# and exactly the value that overflowed the write.
268+
signatures=[b'\x30' * 73, b'', b''],
269+
m=2,
270+
)
271+
272+
inp1 = proto_types.TxInputType(address_n=[1],
273+
prev_hash=binascii.unhexlify('c6091adf4c0c23982a35899a6e58ae11e703eacd7954f588ed4b9cdefc4dba52'),
274+
prev_index=1,
275+
script_type=proto_types.SPENDMULTISIG,
276+
multisig=multisig,
277+
)
278+
279+
out1 = proto_types.TxOutputType(address='12iyMbUb4R2K3gre4dHSrbu5azG5KaqVss',
280+
amount=100000,
281+
script_type=proto_types.PAYTOADDRESS)
282+
283+
with self.client:
284+
self.assertRaises(CallException, self.client.sign_tx, 'Bitcoin', [inp1, ], [out1, ])
285+
286+
243287
if __name__ == '__main__':
244288
unittest.main()

0 commit comments

Comments
 (0)