← Watch feed
feat(core): sign messages at an export point under any script type
Public commit record
85/100 · Strong
What the developer wrote
Authored by Petr Susil
feat(core): sign messages at an export point under any script type
Signing a message failed unless the host sent the script type matching the
path. Any script type a message can be signed with is now accepted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context
What changed, and why it matters
This commit relaxes a security check in Trezor's Bitcoin message-signing feature. Previously, the device required the host's chosen script type (for example, legacy, SegWit, or native SegWit) to match the exact BIP-32 derivation path. Now, any supported script type is accepted as long as the path reaches a valid 'export point' for one of those script types. The change is described as a feature, not a bug fix, and the tests were rewritten to expect signing to succeed in cases that were previously forbidden or warned. This broadens what a host can ask the device to sign, which could make phishing or UI-bypass attacks slightly easier, but it does not by itself leak keys or allow arbitrary paths.
Recommended action
Treat this as a behavior change that reduces a defensive invariant. Review whether the relaxed policy could be abused to make a user sign a message on a path whose on-screen address type does not match the host's claimed script type, and consider whether the UI now clearly shows the effective address/script type before signing. If this change ships in a release, document it in release notes and, if warranted, request a security review of the new test matrix to ensure no unintended path grants were introduced.
Security signals we found
01
Relaxation of path/script-type binding for SignMessage
02
New helper merges export patterns across multiple InputScriptTypes
03
Tests rewritten to expect success where previous tests expected Forbidden key path
04
SPENDTAPROOT explicitly kept restricted
05
No mention of CVE, advisory, or security bug in commit message
Evidence from the diff
The patch changes core/src/apps/bitcoin/keychain.py so that SignMessage path validation and keychain unlocking use a new _sign_message_export_patterns() helper. That helper returns the union of xpub-export patterns for SPENDADDRESS, SPENDP2SHWITNESS, and SPENDWITNESS, regardless of the script type in the actual message. As a result, a path that is a valid export point for any of those three script types is now accepted for message signing under any of the three script types. The old per-script-type check is removed. Tests are updated: many cases formerly marked FORBIDDEN now expect SIGNS, and some formerly WARN cases now expect no warning. The commit explicitly excludes SPENDTAPROOT and keeps taproot paths behind UnlockPath.
Changed components
core/src/apps/bitcoin/keychain.pycore/tests/test_apps.bitcoin.keychain.pytests/device_tests/bitcoin/test_signmessage.pyInspect captured patch +137 / −129
### core/src/apps/bitcoin/keychain.py
@@ -178,7 +178,7 @@ def validate_path_against_script_type(
if SignMessage.is_type_of(msg):
patterns += _get_patterns_for_script_type(coin, script_type, multisig=True)
- patterns += _xpub_export_patterns(coin, script_type)
+ patterns += _sign_message_export_patterns(coin)
return any(
PathSchema.parse(pattern, coin.slip44).match(address_n) for pattern in patterns
@@ -221,6 +221,21 @@ def _xpub_export_patterns(
return export_patterns
+def _sign_message_export_patterns(coin: coininfo.CoinInfo) -> list[str]:
+ """Export points of every script type sign_message() can sign with."""
+ patterns: list[str] = []
+ for script_type in (
+ InputScriptType.SPENDADDRESS,
+ InputScriptType.SPENDP2SHWITNESS,
+ InputScriptType.SPENDWITNESS,
+ ):
+ for pattern in _xpub_export_patterns(coin, script_type):
+ if pattern not in patterns:
+ patterns.append(pattern)
+
+ return patterns
+
+
def validate_xpub_path_against_script_type(
coin: coininfo.CoinInfo,
address_n: Bip32Path,
@@ -385,20 +400,12 @@ async def wrapper(
coin = _get_coin_by_name(msg.coin_name)
extra_schemas = _get_unlock_schemas(msg, auth_msg, coin)
if SignMessage.is_type_of(msg):
- script_type = msg.script_type or InputScriptType.SPENDADDRESS
# Only the export points need granting; leaf patterns are already
- # in _get_schemas_for_coin(). SPENDTAPROOT is out, which keeps
- # PATTERN_SLIP25_TAPROOT behind UnlockPath.
- if script_type in (
- InputScriptType.SPENDADDRESS,
- InputScriptType.SPENDP2SHWITNESS,
- InputScriptType.SPENDWITNESS,
- ):
- # Do not add Bitcoin-path aliases for fork coins.
- extra_schemas += [
- PathSchema.parse(pattern, coin.slip44)
- for pattern in _xpub_export_patterns(coin, script_type)
- ]
+ # in _get_schemas_for_coin(), and no Bitcoin-path aliases for forks.
+ extra_schemas += [
+ PathSchema.parse(pattern, coin.slip44)
+ for pattern in _sign_message_export_patterns(coin)
+ ]
keychain = await _get_keychain_for_coin(coin, extra_schemas)
if AuthorizeCoinJoin.is_type_of(auth_msg):
auth_obj = authorization.from_cached_message(auth_msg)
### core/tests/test_apps.bitcoin.keychain.py
@@ -271,17 +271,30 @@ def test_account_node_and_leaf(self):
self.assertTrue(validate_path_against_script_type(coin, msg))
self.assertTrue(self._derive(msg))
- def test_script_type_must_match_the_level(self):
- """Only the matching account node is granted, so this is not derivable."""
+ def test_export_point_ignores_the_script_type(self):
+ from trezor.enums import InputScriptType
+
+ coin = _get_coin_by_name("Bitcoin")
+ for script_type in (
+ InputScriptType.SPENDADDRESS,
+ InputScriptType.SPENDP2SHWITNESS,
+ InputScriptType.SPENDWITNESS,
+ ):
+ msg = self._sign_message([H_(48), H_(0), H_(0), H_(2)], script_type)
+ self.assertTrue(validate_path_against_script_type(coin, msg))
+ self.assertTrue(self._derive(msg))
+
+ def test_leaf_script_type_must_match_the_level(self):
+ """Below an export point the encoding must match, but only warns."""
from trezor.enums import InputScriptType
coin = _get_coin_by_name("Bitcoin")
msg = self._sign_message(
- [H_(48), H_(0), H_(0), H_(2)], InputScriptType.SPENDADDRESS
+ [H_(48), H_(0), H_(0), H_(2), 0, 0], InputScriptType.SPENDADDRESS
)
self.assertFalse(validate_path_against_script_type(coin, msg))
- self.assertRaises(wire.DataError, self._derive, msg)
+ self.assertTrue(self._derive(msg))
def test_fork_coins_get_no_bitcoin_path_alias(self):
"""The Bitcoin-namespace alias of a fork must not carry the grant.
### tests/device_tests/bitcoin/test_signmessage.py
@@ -495,85 +495,52 @@ def test_signmessage_path_warning(session: Session):
)
-# BIP-48 message signing, see #7717. The whole permission surface, one
-# trezorctl invocation per row:
-#
-# trezorctl btc sign-message -n "<path>" -t <script type> "hello"
-
MESSAGE = "This is an example of a signed message."
-# Not in the keychain, so refused outright rather than warned about.
-FORBIDDEN = "forbidden"
-# Signs, but shows the unknown derivation path screen first.
-WARN = "warn"
-# Signs with no warning.
-SIGNS = "signs"
-
-VECTORS_BIP48_MATRIX = ( # path, script_type, expected
+VECTORS_BIP48_SIGNS = ( # path, script_type
# 0h is the legacy-multisig level; message signing is single-key, so it
# signs as p2pkh and trezorctl sends SPENDADDRESS rather than SPENDMULTISIG
- pytest.param("m/48h/0h/0h/0h", S.SPENDADDRESS, SIGNS, id="account_0h-address"),
- pytest.param(
- "m/48h/0h/0h/0h", S.SPENDP2SHWITNESS, FORBIDDEN, id="account_0h-p2shsegwit"
- ),
- pytest.param("m/48h/0h/0h/0h", S.SPENDWITNESS, FORBIDDEN, id="account_0h-segwit"),
- pytest.param("m/48h/0h/0h/0h/0/0", S.SPENDADDRESS, SIGNS, id="leaf_0h-address"),
- pytest.param(
- "m/48h/0h/0h/0h/0/0", S.SPENDP2SHWITNESS, WARN, id="leaf_0h-p2shsegwit"
- ),
- pytest.param("m/48h/0h/0h/0h/0/0", S.SPENDWITNESS, WARN, id="leaf_0h-segwit"),
+ pytest.param("m/48h/0h/0h/0h", S.SPENDADDRESS, id="account_0h-address"),
+ pytest.param("m/48h/0h/0h/0h", S.SPENDP2SHWITNESS, id="account_0h-p2shsegwit"),
+ pytest.param("m/48h/0h/0h/0h", S.SPENDWITNESS, id="account_0h-segwit"),
+ pytest.param("m/48h/0h/0h/0h/0/0", S.SPENDADDRESS, id="leaf_0h-address"),
# 1h is the P2SH-segwit level
- pytest.param("m/48h/0h/0h/1h", S.SPENDADDRESS, FORBIDDEN, id="account_1h-address"),
- pytest.param(
- "m/48h/0h/0h/1h", S.SPENDP2SHWITNESS, SIGNS, id="account_1h-p2shsegwit"
- ),
- pytest.param("m/48h/0h/0h/1h", S.SPENDWITNESS, FORBIDDEN, id="account_1h-segwit"),
- pytest.param("m/48h/0h/0h/1h/0/0", S.SPENDADDRESS, WARN, id="leaf_1h-address"),
- pytest.param(
- "m/48h/0h/0h/1h/0/0", S.SPENDP2SHWITNESS, SIGNS, id="leaf_1h-p2shsegwit"
- ),
- pytest.param("m/48h/0h/0h/1h/0/0", S.SPENDWITNESS, WARN, id="leaf_1h-segwit"),
+ pytest.param("m/48h/0h/0h/1h", S.SPENDADDRESS, id="account_1h-address"),
+ pytest.param("m/48h/0h/0h/1h", S.SPENDP2SHWITNESS, id="account_1h-p2shsegwit"),
+ pytest.param("m/48h/0h/0h/1h", S.SPENDWITNESS, id="account_1h-segwit"),
+ pytest.param("m/48h/0h/0h/1h/0/0", S.SPENDP2SHWITNESS, id="leaf_1h-p2shsegwit"),
# 2h is the native-segwit level
- pytest.param("m/48h/0h/0h/2h", S.SPENDADDRESS, FORBIDDEN, id="account_2h-address"),
- pytest.param(
- "m/48h/0h/0h/2h", S.SPENDP2SHWITNESS, FORBIDDEN, id="account_2h-p2shsegwit"
- ),
- pytest.param("m/48h/0h/0h/2h", S.SPENDWITNESS, SIGNS, id="account_2h-segwit"),
- pytest.param("m/48h/0h/0h/2h/0/0", S.SPENDADDRESS, WARN, id="leaf_2h-address"),
- pytest.param(
- "m/48h/0h/0h/2h/0/0", S.SPENDP2SHWITNESS, WARN, id="leaf_2h-p2shsegwit"
- ),
- pytest.param("m/48h/0h/0h/2h/0/0", S.SPENDWITNESS, SIGNS, id="leaf_2h-segwit"),
+ pytest.param("m/48h/0h/0h/2h", S.SPENDADDRESS, id="account_2h-address"),
+ pytest.param("m/48h/0h/0h/2h", S.SPENDP2SHWITNESS, id="account_2h-p2shsegwit"),
+ pytest.param("m/48h/0h/0h/2h", S.SPENDWITNESS, id="account_2h-segwit"),
+ pytest.param("m/48h/0h/0h/2h/0/0", S.SPENDWITNESS, id="leaf_2h-segwit"),
+)
+VECTORS_BIP48_WARN = ( # path, script_type
+ # A leaf whose script type does not match the level it hangs under is
+ # below the export point, so it warns before signing
+ pytest.param("m/48h/0h/0h/0h/0/0", S.SPENDP2SHWITNESS, id="leaf_0h-p2shsegwit"),
+ pytest.param("m/48h/0h/0h/0h/0/0", S.SPENDWITNESS, id="leaf_0h-segwit"),
+ pytest.param("m/48h/0h/0h/1h/0/0", S.SPENDADDRESS, id="leaf_1h-address"),
+ pytest.param("m/48h/0h/0h/1h/0/0", S.SPENDWITNESS, id="leaf_1h-segwit"),
+ pytest.param("m/48h/0h/0h/2h/0/0", S.SPENDADDRESS, id="leaf_2h-address"),
+ pytest.param("m/48h/0h/0h/2h/0/0", S.SPENDP2SHWITNESS, id="leaf_2h-p2shsegwit"),
)
-def _assert_sign_message(
+def _sign_message(
session: Session,
path: str,
script_type: messages.InputScriptType,
- expected: str,
+ warns: bool,
):
- if expected == FORBIDDEN:
- with pytest.raises(TrezorFailure, match="Forbidden key path") as exc:
- btc.sign_message(
- session,
- coin_name="Bitcoin",
- n=parse_path(path),
- message=MESSAGE,
- script_type=script_type,
- )
- assert exc.value.code is messages.FailureType.DataError
- return
-
- expected_responses = []
- if expected == WARN:
- expected_responses.append(
+ expected_responses = [
+ (
+ warns,
message_filters.ButtonRequest(
code=messages.ButtonRequestType.UnknownDerivationPath
- )
- )
- expected_responses += [
+ ),
+ ),
message_filters.ButtonRequest(code=messages.ButtonRequestType.Other),
message_filters.ButtonRequest(code=messages.ButtonRequestType.Other),
messages.MessageSignature,
@@ -593,57 +560,97 @@ def _assert_sign_message(
)
assert sig.signature
+ return sig
@pytest.mark.models("core")
-@pytest.mark.parametrize("path, script_type, expected", VECTORS_BIP48_MATRIX)
-def test_signmessage_bip48_matrix(
+@pytest.mark.parametrize("path, script_type", VECTORS_BIP48_SIGNS)
+def test_signmessage_bip48_matrix_no_warning(
session: Session,
path: str,
script_type: messages.InputScriptType,
- expected: str,
):
- _assert_sign_message(session, path, script_type, expected)
+ _sign_message(session, path, script_type, warns=False)
-VECTORS_SIGNMESSAGE_LENIENCY = ( # path, script_type, expected
+@pytest.mark.models("core")
+@pytest.mark.parametrize("path, script_type", VECTORS_BIP48_WARN)
+def test_signmessage_bip48_matrix_warning(
+ session: Session,
+ path: str,
+ script_type: messages.InputScriptType,
+):
+ _sign_message(session, path, script_type, warns=True)
+
+
+VECTORS_LENIENCY_SIGNS = ( # path, script_type
# The BIP-45 cosigner node: PATTERN_BIP45 is unhardened below m/45'
- pytest.param("m/45h", S.SPENDADDRESS, SIGNS, id="cosigner_node-address"),
- pytest.param("m/45h", S.SPENDP2SHWITNESS, SIGNS, id="cosigner_node-p2shsegwit"),
- pytest.param("m/45h", S.SPENDWITNESS, SIGNS, id="cosigner_node-segwit"),
- pytest.param("m/45h/0/0/0", S.SPENDADDRESS, SIGNS, id="bip45_leaf"),
+ pytest.param("m/45h", S.SPENDADDRESS, id="cosigner_node-address"),
+ pytest.param("m/45h", S.SPENDP2SHWITNESS, id="cosigner_node-p2shsegwit"),
+ pytest.param("m/45h", S.SPENDWITNESS, id="cosigner_node-segwit"),
+ pytest.param("m/45h/0/0/0", S.SPENDADDRESS, id="bip45_leaf"),
# The Casa and Unchained-unhardened account level
- pytest.param("m/45h/0/0", S.SPENDADDRESS, SIGNS, id="casa_account-address"),
- pytest.param("m/45h/0/0", S.SPENDP2SHWITNESS, SIGNS, id="casa_account-p2shsegwit"),
- pytest.param("m/45h/0/0", S.SPENDWITNESS, SIGNS, id="casa_account-segwit"),
+ pytest.param("m/45h/0/0", S.SPENDADDRESS, id="casa_account-address"),
+ pytest.param("m/45h/0/0", S.SPENDP2SHWITNESS, id="casa_account-p2shsegwit"),
+ pytest.param("m/45h/0/0", S.SPENDWITNESS, id="casa_account-segwit"),
# The Unchained hardened account level
- pytest.param("m/45h/0h/0h", S.SPENDADDRESS, SIGNS, id="unchained_account-address"),
- pytest.param(
- "m/45h/0h/0h", S.SPENDP2SHWITNESS, FORBIDDEN, id="unchained_account-p2shsegwit"
- ),
- pytest.param("m/45h/0h/0h", S.SPENDWITNESS, SIGNS, id="unchained_account-segwit"),
+ pytest.param("m/45h/0h/0h", S.SPENDADDRESS, id="unchained_account-address"),
+ pytest.param("m/45h/0h/0h", S.SPENDP2SHWITNESS, id="unchained_account-p2shsegwit"),
+ pytest.param("m/45h/0h/0h", S.SPENDWITNESS, id="unchained_account-segwit"),
# Unchained, whose account level is hardened
- pytest.param("m/45h/0h/0h/1000000/0/0", S.SPENDADDRESS, SIGNS, id="unchained_leaf"),
+ pytest.param("m/45h/0h/0h/1000000/0/0", S.SPENDADDRESS, id="unchained_leaf"),
# An ordinary account node, where any wallet shares its xpub
- pytest.param("m/44h/0h/0h", S.SPENDADDRESS, SIGNS, id="bip44_account"),
- pytest.param("m/84h/0h/0h", S.SPENDWITNESS, SIGNS, id="bip84_account"),
- # The script type still has to match the purpose
- pytest.param("m/44h/0h/0h", S.SPENDWITNESS, FORBIDDEN, id="bip44_account-segwit"),
+ pytest.param("m/44h/0h/0h", S.SPENDADDRESS, id="bip44_account"),
+ pytest.param("m/84h/0h/0h", S.SPENDWITNESS, id="bip84_account"),
+ # An export point is reached under any signable script type
+ pytest.param("m/44h/0h/0h", S.SPENDWITNESS, id="bip44_account-segwit"),
+)
+VECTORS_LENIENCY_FORBIDDEN = ( # path, script_type
# A pattern with no hardened component has no export point, so the root
# stays withheld
- pytest.param("m", S.SPENDADDRESS, FORBIDDEN, id="root"),
+ pytest.param("m", S.SPENDADDRESS, id="root"),
+ # The same holds deeper down: PATTERN_CASA_UNHARDENED is unhardened
+ # throughout, so its account level is withheld even though its leaves sign
+ pytest.param("m/49/0/0", S.SPENDP2SHWITNESS, id="casa_unhardened_account"),
+ # Above the export point
+ pytest.param("m/44h/0h", S.SPENDADDRESS, id="bip44_coin_type"),
+ # Below the export point, but not a leaf: the grant is the node itself,
+ # not its subtree
+ pytest.param("m/44h/0h/0h/0", S.SPENDADDRESS, id="bip44_change"),
+ # The export point of the right shape, but under the wrong coin_type
+ pytest.param("m/44h/1h/0h", S.SPENDADDRESS, id="bip44_account-testnet"),
+ # A message cannot be signed with SPENDTAPROOT, so no taproot export point
+ # is granted, not even for a script type that could sign
+ pytest.param("m/86h/0h/0h", S.SPENDADDRESS, id="bip86_account"),
)
@pytest.mark.models("core")
-@pytest.mark.parametrize("path, script_type, expected", VECTORS_SIGNMESSAGE_LENIENCY)
+@pytest.mark.parametrize("path, script_type", VECTORS_LENIENCY_SIGNS)
def test_signmessage_leniency(
session: Session,
path: str,
script_type: messages.InputScriptType,
- expected: str,
):
- _assert_sign_message(session, path, script_type, expected)
+ _sign_message(session, path, script_type, warns=False)
+
+
+@pytest.mark.models("core")
+@pytest.mark.parametrize("path, script_type", VECTORS_LENIENCY_FORBIDDEN)
+def test_signmessage_forbidden(
+ session: Session,
+ path: str,
+ script_type: messages.InputScriptType,
+):
+ with pytest.raises(TrezorFailure, match="Forbidden key path") as e:
+ btc.sign_message(
+ session,
+ coin_name="Bitcoin",
+ n=parse_path(path),
+ message=MESSAGE,
+ script_type=script_type,
+ )
+ assert e.value.code is messages.FailureType.DataError
@pytest.mark.models("core")
@@ -652,29 +659,10 @@ def test_signmessage_bip48_legacy_level_signs_as_p2pkh(session: Session):
# signature, so trezorctl sends the single-key analogue.
from trezorlib.cli.btc import guess_script_type_from_path
- address_n = parse_path("m/48h/0h/0h/0h/0/0")
- assert guess_script_type_from_path(address_n) is S.SPENDMULTISIG
-
- expected_responses = [
- message_filters.ButtonRequest(code=messages.ButtonRequestType.Other),
- message_filters.ButtonRequest(code=messages.ButtonRequestType.Other),
- messages.MessageSignature,
- ]
-
- with session.test_ctx as client:
- client.set_expected_responses(expected_responses)
- if is_core(session):
- IF = InputFlowConfirmAllWarnings(session)
- client.set_input_flow(IF.get())
- sig = btc.sign_message(
- session,
- coin_name="Bitcoin",
- n=address_n,
- message=MESSAGE,
- script_type=S.SPENDADDRESS,
- )
+ path = "m/48h/0h/0h/0h/0/0"
+ assert guess_script_type_from_path(parse_path(path)) is S.SPENDMULTISIG
- assert sig.signature
+ sig = _sign_message(session, path, S.SPENDADDRESS, warns=False)
assert sig.address.startswith("1")
Risk score
Our methodology →Why this scored 37/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.