fix(core): disallow per-node paths in getaddress

core/src/apps/bitcoin/common.py

@@ -5,6 +5,7 @@ from trezor import wire
 from trezor.crypto import bech32
 from trezor.crypto.curve import bip340
 from trezor.enums import InputScriptType, OutputScriptType
+from trezor.messages import MultisigRedeemScriptType
 
 if TYPE_CHECKING:
     from enum import IntEnum
@@ -263,3 +264,11 @@ def descriptor_checksum(desc: str) -> str:
     for j in range(0, 8):
         ret[j] = CHECKSUM_CHARSET[(c >> (5 * (7 - j))) & 31]
     return "".join(ret)
+
+
+def multisig_uses_single_path(multisig: MultisigRedeemScriptType) -> bool:
+    if multisig.pubkeys is not None:
+        return all(
+            [hd.address_n == multisig.pubkeys[0].address_n for hd in multisig.pubkeys]
+        )
+    return True

core/src/apps/bitcoin/get_address.py

@@ -2,6 +2,9 @@ from typing import TYPE_CHECKING
 
 from trezor.enums import MultisigPubkeysOrder
 
+from apps.common import safety_checks
+
+from .common import multisig_uses_single_path
 from .keychain import with_keychain
 
 if TYPE_CHECKING:
@@ -35,7 +38,11 @@ def _get_xpubs(
 async def get_address(msg: GetAddress, keychain: Keychain, coin: CoinInfo) -> Address:
     from trezor.enums import InputScriptType
     from trezor.messages import Address
-    from trezor.ui.layouts import confirm_multisig_warning, show_address
+    from trezor.ui.layouts import (
+        confirm_multisig_different_paths_warning,
+        confirm_multisig_warning,
+        show_address,
+    )
 
     from apps.common.address_mac import get_address_mac
     from apps.common.paths import address_n_to_str, validate_path
@@ -107,6 +114,15 @@ async def get_address(msg: GetAddress, keychain: Keychain, coin: CoinInfo) -> Ad
 
             await confirm_multisig_warning()
 
+            # An addresss that uses different derivation paths for different xpubs could be difficult to discover
+            if not multisig_uses_single_path(multisig):
+                if safety_checks.is_strict():
+                    raise ValueError(
+                        "Using different paths for different xpubs is not allowed"
+                    )
+                else:
+                    await confirm_multisig_different_paths_warning()
+
             if multisig.pubkeys_order == MultisigPubkeysOrder.LEXICOGRAPHIC:
                 account = f"Multisig {multisig.m} of {len(pubnodes)}\n(sorted)"
             else:

core/src/trezor/ui/layouts/mercury/__init__.py

@@ -139,6 +139,17 @@ def confirm_multisig_warning() -> Awaitable[None]:
     )
 
 
+def confirm_multisig_different_paths_warning() -> Awaitable[None]:
+    return raise_if_not_confirmed(
+        trezorui2.flow_warning_hi_prio(
+            title=f"{TR.words__important}!",
+            description="Using different paths for different xpubs.",
+        ),
+        "warning_multisig_different_paths",
+        br_code=ButtonRequestType.Warning,
+    )
+
+
 def confirm_homescreen(
     image: bytes,
 ) -> Awaitable[None]:

core/src/trezor/ui/layouts/tr/__init__.py

@@ -181,6 +181,14 @@ def confirm_multisig_warning() -> Awaitable[ui.UiResult]:
     )
 
 
+def confirm_multisig_different_paths_warning() -> Awaitable[ui.UiResult]:
+    return show_warning(
+        "warning_multisig_different_paths",
+        "Using different paths for different xpubs.",
+        TR.words__continue_anyway_question,
+    )
+
+
 def confirm_homescreen(image: bytes) -> Awaitable[None]:
     return raise_if_not_confirmed(
         trezorui2.confirm_homescreen(

core/src/trezor/ui/layouts/tt/__init__.py

@@ -162,6 +162,14 @@ def confirm_multisig_warning() -> Awaitable[None]:
     )
 
 
+def confirm_multisig_different_paths_warning() -> Awaitable[None]:
+    return show_warning(
+        "warning_multisig_different_paths",
+        "Using different paths for different xpubs.",
+        TR.words__continue_anyway_question,
+    )
+
+
 def confirm_homescreen(image: bytes) -> Awaitable[None]:
     return raise_if_not_confirmed(
         trezorui2.confirm_homescreen(

tests/device_tests/bitcoin/test_getaddress.py

@@ -435,3 +435,51 @@ def test_crw(client: Client):
         btc.get_address(client, "Crown", parse_path("m/44h/72h/0h/0/0"))
         == "CRWYdvZM1yXMKQxeN3hRsAbwa7drfvTwys48"
     )
+
+
+@pytest.mark.multisig
+@pytest.mark.models(skip="legacy", reason="Not fixed")
+def test_multisig_different_paths(client: Client):
+    nodes = [
+        btc.get_public_node(client, parse_path(f"m/45h/{i}"), coin_name="Bitcoin").node
+        for i in range(2)
+    ]
+
+    multisig = messages.MultisigRedeemScriptType(
+        pubkeys=[
+            messages.HDNodePathType(node=node, address_n=[0, i])
+            for i, node in enumerate(nodes)
+        ],
+        signatures=[b"", b"", b""],
+        m=2,
+    )
+
+    with pytest.raises(
+        Exception, match="Using different paths for different xpubs is not allowed"
+    ):
+        with client:
+            if is_core(client):
+                IF = InputFlowConfirmAllWarnings(client)
+                client.set_input_flow(IF.get())
+            btc.get_address(
+                client,
+                "Bitcoin",
+                parse_path("m/45h/0/0/0"),
+                show_display=True,
+                multisig=multisig,
+                script_type=messages.InputScriptType.SPENDMULTISIG,
+            )
+
+    device.apply_settings(client, safety_checks=SafetyCheckLevel.PromptTemporarily)
+    with client:
+        if is_core(client):
+            IF = InputFlowConfirmAllWarnings(client)
+            client.set_input_flow(IF.get())
+        btc.get_address(
+            client,
+            "Bitcoin",
+            parse_path("m/45h/0/0/0"),
+            show_display=True,
+            multisig=multisig,
+            script_type=messages.InputScriptType.SPENDMULTISIG,
+        )

tests/input_flows.py

@@ -2273,6 +2273,7 @@ class InputFlowConfirmAllWarnings(InputFlowBase):
                     "witness path",
                     "certificate path",
                     "pool owner staking path",
+                    "using different paths for different xpubs",
                 ]
             )
             if any(needle.lower() in text for needle in hi_prio):