fix(core): don't reuse `FORBIDDEN_KEY_PATH` exception object
What the developer wrote
Authored by Roman Zeyde
fix(core): don't reuse `FORBIDDEN_KEY_PATH` exception object
Following https://github.com/trezor/trezor-firmware/issues/5472#issuecomment-3174382038.
[no changelog]
What changed, and why it matters
This commit fixes a subtle bug where the same error object was reused every time a forbidden key path was accessed. In Python/MicroPython, reusing a single exception instance can cause problems if something later modifies or inspects the raised error, because the same object carries state from one error site to another. The fix creates a fresh exception object each time, which is safer and more correct, but the commit message and diff do not clearly describe an active security vulnerability.
Treat as a low-risk defensive fix. Review the linked GitHub issue comment for any additional context, and verify that no other singleton exception objects are reused elsewhere in the firmware. No urgent user action is indicated.
Security signals we found
Reused mutable exception singleton replaced with per-call instance creation
Change affects path validation and key derivation access control
References GitHub issue comment, suggesting prior discussion of the pattern
No changelog entry, consistent with minor/internal fix
Evidence from the diff
The change replaces a module-level singleton FORBIDDEN_KEY_PATH = DataError(“Forbidden key path”) with a ForbiddenKeyPath class whose instances are raised fresh at each call site. In MicroPython, raising the same exception object repeatedly can lead to unexpected behavior if the exception’s traceback or internal state is mutated or preserved across raises. The patch touches three files: authorize_coinjoin.py, get_public_key.py, and keychain.py. It is a defensive correctness fix rather than a patch for a demonstrated exploit.
Changed components
core/src/apps/common/keychain.pycore/src/apps/bitcoin/authorize_coinjoin.pycore/src/apps/bitcoin/get_public_key.pyInspect captured patch +10 / −8
diff --git a/core/src/apps/bitcoin/authorize_coinjoin.py b/core/src/apps/bitcoin/authorize_coinjoin.py
index cc43f258..f2c934c5 100644
--- a/core/src/apps/bitcoin/authorize_coinjoin.py
+++ b/core/src/apps/bitcoin/authorize_coinjoin.py
@@ -26,7 +26,7 @@ async def authorize_coinjoin(
from trezor.wire import DataError
from apps.common import authorization, safety_checks
- from apps.common.keychain import FORBIDDEN_KEY_PATH
+ from apps.common.keychain import ForbiddenKeyPath
from apps.common.paths import SLIP25_PURPOSE, validate_path
from .common import BIP32_WALLET_DEPTH, format_fee_rate
@@ -59,7 +59,7 @@ async def authorize_coinjoin(
raise DataError("Empty path not allowed.")
if address_n[0] != SLIP25_PURPOSE and safety_checks_is_strict:
- raise FORBIDDEN_KEY_PATH
+ raise ForbiddenKeyPath()
max_fee_per_vbyte = format_fee_rate(
msg.max_fee_per_kvbyte / 1000, coin, include_shortcut=True
diff --git a/core/src/apps/bitcoin/get_public_key.py b/core/src/apps/bitcoin/get_public_key.py
index b750d8d6..30130184 100644
--- a/core/src/apps/bitcoin/get_public_key.py
+++ b/core/src/apps/bitcoin/get_public_key.py
@@ -19,7 +19,7 @@ async def get_public_key(
from trezor.messages import HDNodeType, PublicKey, UnlockPath
from apps.common import coininfo, paths
- from apps.common.keychain import FORBIDDEN_KEY_PATH, get_keychain
+ from apps.common.keychain import ForbiddenKeyPath, get_keychain
coin_name = msg.coin_name or "Bitcoin"
script_type = msg.script_type or InputScriptType.SPENDADDRESS
@@ -32,11 +32,11 @@ async def get_public_key(
if address_n and address_n[0] == paths.SLIP25_PURPOSE:
# UnlockPath is required to access SLIP25 paths.
if not UnlockPath.is_type_of(auth_msg):
- raise FORBIDDEN_KEY_PATH
+ raise ForbiddenKeyPath()
# Verify that the desired path lies in the unlocked subtree.
if auth_msg.address_n != address_n[: len(auth_msg.address_n)]:
- raise FORBIDDEN_KEY_PATH
+ raise ForbiddenKeyPath()
if not keychain:
keychain = await get_keychain(curve_name, [paths.AlwaysMatchingSchema])
diff --git a/core/src/apps/common/keychain.py b/core/src/apps/common/keychain.py
index 5120f694..4defb14a 100644
--- a/core/src/apps/common/keychain.py
+++ b/core/src/apps/common/keychain.py
@@ -34,7 +34,9 @@ if TYPE_CHECKING:
def __del__(self) -> None: ...
-FORBIDDEN_KEY_PATH = DataError("Forbidden key path")
+class ForbiddenKeyPath(DataError):
+ def __init__(self) -> None:
+ super().__init__("Forbidden key path")
class LRUCache:
@@ -103,7 +105,7 @@ class Keychain:
if self.is_in_keychain(path):
return
- raise FORBIDDEN_KEY_PATH
+ raise ForbiddenKeyPath()
def is_in_keychain(self, path: paths.Bip32Path) -> bool:
return any(schema.match(path) for schema in self.schemas)
@@ -150,7 +152,7 @@ class Keychain:
if safety_checks.is_strict() and not any(
ns == path[: len(ns)] for ns in self.slip21_namespaces
):
- raise FORBIDDEN_KEY_PATH
+ raise ForbiddenKeyPath()
return self._derive_with_cache(
1,
Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.