From b7d26c794b8839fb9a35208d3b7a4ae336424480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Vejpustek?= Date: Thu, 28 Nov 2024 13:56:36 +0100 Subject: [PATCH] fixup! feat(core)): forbid multisig to singlesig change outputs --- .../apps/bitcoin/sign_tx/change_detector.py | 35 +++++++------------ core/src/apps/bitcoin/sign_tx/matchcheck.py | 11 +++--- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/core/src/apps/bitcoin/sign_tx/change_detector.py b/core/src/apps/bitcoin/sign_tx/change_detector.py index ae8995846f..3f88e52c5e 100644 --- a/core/src/apps/bitcoin/sign_tx/change_detector.py +++ b/core/src/apps/bitcoin/sign_tx/change_detector.py @@ -17,15 +17,11 @@ _BIP32_MAX_LAST_ELEMENT = const(1_000_000) class ChangeDetector: def __init__(self) -> None: from .matchcheck import ( - MultisigChecker, MultisigFingerprintChecker, ScriptTypeChecker, WalletPathChecker, ) - # Whether all inputs are multisig or all inputs are singlesig, used to validate change-output. - self.multisig = MultisigChecker() - # Checksum of multisig inputs, used to validate change-output. self.multisig_fingerprint = MultisigFingerprintChecker() @@ -40,37 +36,30 @@ class ChangeDetector: self.wallet_path.add_input(txi) self.script_type.add_input(txi) self.multisig_fingerprint.add_input(txi) - self.multisig.add_input(txi) def check_input(self, txi: TxInput) -> None: self.wallet_path.check_input(txi) self.script_type.check_input(txi) self.multisig_fingerprint.check_input(txi) - self.multisig.check_input(txi) def output_is_change(self, txo: TxOutput) -> bool: if txo.script_type not in common.CHANGE_OUTPUT_SCRIPT_TYPES: return False - if txo.multisig: - if not ( - self.multisig_fingerprint.output_matches(txo) - and 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 + 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.output_matches(txo) + self.multisig_fingerprint.output_matches(txo) and self.wallet_path.output_matches(txo) and self.script_type.output_matches(txo) and len(txo.address_n) >= common.BIP32_WALLET_DEPTH diff --git a/core/src/apps/bitcoin/sign_tx/matchcheck.py b/core/src/apps/bitcoin/sign_tx/matchcheck.py index 3cedb75034..1acb812cef 100644 --- a/core/src/apps/bitcoin/sign_tx/matchcheck.py +++ b/core/src/apps/bitcoin/sign_tx/matchcheck.py @@ -103,15 +103,14 @@ class MultisigFingerprintChecker(MatchChecker): from .. import multisig if not txio.multisig: - return None + # The fingerprint of a singlesig input or output is defined as an empty byte string. + # This has two consequences: First, a singlesig output matches if and only if all + # the added inputs are singlesig. Second, a multisig output does not match if any of + # the added inputs is singlesig. + return bytes() return multisig.multisig_fingerprint(txio.multisig) -class MultisigChecker(MatchChecker): - def attribute_from_tx(self, txio: TxInput | TxOutput) -> Any: - return txio.multisig is not None - - class ScriptTypeChecker(MatchChecker): def attribute_from_tx(self, txio: TxInput | TxOutput) -> Any: from trezor.enums import InputScriptType