Skip to content

Commit

Permalink
fix(core): disallow per-node paths in change outputs
Browse files Browse the repository at this point in the history
  • Loading branch information
onvej-sl committed Dec 2, 2024
1 parent 45a029e commit ae32a23
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 0 deletions.
1 change: 1 addition & 0 deletions core/.changelog.d/4351.changed.3
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Forbid per-node paths in multisig change outputs and multisig receive addresses.
12 changes: 12 additions & 0 deletions core/src/apps/bitcoin/sign_tx/change_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ def output_is_change(self, txo: TxOutput) -> bool:
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)
Expand Down
37 changes: 37 additions & 0 deletions tests/device_tests/bitcoin/test_multisig_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit ae32a23

Please sign in to comment.