diff --git a/core/.changelog.d/4351.changed.3 b/core/.changelog.d/4351.changed.3 new file mode 100644 index 0000000000..87cd7c8ae5 --- /dev/null +++ b/core/.changelog.d/4351.changed.3 @@ -0,0 +1 @@ +Forbid per-node paths in multisig change outputs and multisig receive addresses. diff --git a/core/src/apps/bitcoin/sign_tx/change_detector.py b/core/src/apps/bitcoin/sign_tx/change_detector.py index b668206437..3f88e52c5e 100644 --- a/core/src/apps/bitcoin/sign_tx/change_detector.py +++ b/core/src/apps/bitcoin/sign_tx/change_detector.py @@ -46,6 +46,18 @@ class ChangeDetector: if txo.script_type not in common.CHANGE_OUTPUT_SCRIPT_TYPES: return False + if txo.multisig and not common.multisig_uses_single_path(txo.multisig): + # An address that uses different derivation paths for different xpubs + # could be difficult to discover if the user did not note all the paths. + # The reason is that each path ends with an address index, which can + # have 1,000,000 possible values. If the address is a t-out-of-n + # multisig, the total number of possible paths is 1,000,000^n. This can + # be exploited by an attacker who has compromised the user's computer. + # The attacker could randomize the address indices and then demand a + # ransom from the user to reveal the paths. To prevent this, we require + # that all xpubs use the same derivation path. + return False + return ( self.multisig_fingerprint.output_matches(txo) and self.wallet_path.output_matches(txo) diff --git a/tests/device_tests/bitcoin/test_multisig_change.py b/tests/device_tests/bitcoin/test_multisig_change.py index 990c92c82e..9703a9b672 100644 --- a/tests/device_tests/bitcoin/test_multisig_change.py +++ b/tests/device_tests/bitcoin/test_multisig_change.py @@ -406,6 +406,43 @@ def test_multisig_mismatch_multisig_change(client: Client): ) +# inputs match, change mismatches (second tries to be change but isn't) +@pytest.mark.models(skip="legacy", reason="Not fixed") +def test_multisig_mismatch_multisig_change_different_paths(client: Client): + multisig_out2 = messages.MultisigRedeemScriptType( + pubkeys=[ + messages.HDNodePathType(node=NODE_EXT1, address_n=[1, 0]), + messages.HDNodePathType(node=NODE_EXT2, address_n=[1, 1]), + messages.HDNodePathType(node=NODE_INT, address_n=[1, 2]), + ], + signatures=[b"", b"", b""], + m=2, + ) + + out1 = messages.TxOutputType( + address="3B23k4kFBRtu49zvpG3Z9xuFzfpHvxBcwt", + amount=40_000_000, + script_type=messages.OutputScriptType.PAYTOADDRESS, + ) + + out2 = messages.TxOutputType( + address_n=[H_(45), 0, 1, 2], + multisig=multisig_out2, + amount=44_000_000, + script_type=messages.OutputScriptType.PAYTOMULTISIG, + ) + + with client: + client.set_expected_responses(_responses(client, INP1, INP2)) + btc.sign_tx( + client, + "Bitcoin", + [INP1, INP2], + [out1, out2], + prev_txes=TX_API, + ) + + # inputs mismatch, change matches with first input def test_multisig_mismatch_inputs(client: Client): multisig_out1 = messages.MultisigRedeemScriptType(