SFT-981: expose HDNode.blank() again and call it where it was commented out
What changed, and why it matters
This commit fixes a security hygiene issue in the Passport hardware wallet firmware where sensitive cryptographic key material (HD wallet nodes and a BIP39 master seed) was not being actively wiped from device memory when no longer needed. Previously, the wipe function had been removed from the code bindings, leaving a no-op placeholder. The patch restores an explicit 'blank' wipe for HD nodes, calls it in the two places that had TODO comments, adds a finalizer so nodes are also wiped when garbage collected, and wipes a temporary 64-byte master seed that was otherwise never cleared. The risk is that private key material could remain in heap memory longer than intended, potentially increasing exposure if an attacker could read device memory or if memory is reused.
Treat this as a security-hardening fix and include it in the next firmware release. Review other locations where HDNode objects are allocated or converted to ensure all have finalizers and that all temporary seed/secret buffers are explicitly wiped. Consider whether the restored blank() method needs to be guarded behind FOUNDATION_ADDITIONS or can be upstreamed safely.
Security signals we found
Restoration of explicit sensitive-data wiping (HDNode.blank)
Replacement of no-op blank_object() HDNode branch with actual wipe
Addition of finalizer to deserialized HDNode to ensure heap wipe on collection
Wiping of temporary 64-byte BIP39 master seed in SecretStash.decode()
Removal of TODO comments that had replaced actual wipe calls
Addition of unit test validating blanking behavior
Evidence from the diff
The patch re-exposes HDNode.blank() in modtrezorcrypto-bip32.h under FOUNDATION_ADDITIONS, mapping it to the same C function as del, which memzeros the entire hdnode structure including the curve pointer. It updates stash.blank_object() to call item.blank() for HDNode instances instead of doing nothing. It adds blank_object(ms) in SecretStash.decode() to wipe the 64-byte BIP39 master seed after deriving the HD node. It changes bip32.deserialize() to use m_new_obj_with_finaliser so deserialized public nodes are wiped on collection, and memzeros the local hdnode copy. It also replaces two TODO comments in export_summary_flow.py with node.blank() calls before del node. A unit test verifies blanking behavior for private and public nodes and via blank_object().
Changed components
extmod/trezor-firmware/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-bip32.hports/stm32/boards/Passport/modules/stash.pyports/stm32/boards/Passport/modules/flows/export_summary_flow.pyports/stm32/boards/Passport/modules/tests/test_unit.pyports/stm32/boards/Passport/modules/tests/unit/hdnode_blank.pyInspect captured patch +93 / −6
### extmod/trezor-firmware/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-bip32.h
@@ -511,6 +511,12 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_trezorcrypto_HDNode___del___obj,
STATIC const mp_rom_map_elem_t mod_trezorcrypto_HDNode_locals_dict_table[] = {
{MP_ROM_QSTR(MP_QSTR___del__),
MP_ROM_PTR(&mod_trezorcrypto_HDNode___del___obj)},
+#ifdef FOUNDATION_ADDITIONS
+ // Same wipe, reachable before the node is collected. A blanked node keeps no
+ // curve either, so it can only be dropped, never used again.
+ {MP_ROM_QSTR(MP_QSTR_blank),
+ MP_ROM_PTR(&mod_trezorcrypto_HDNode___del___obj)},
+#endif
{MP_ROM_QSTR(MP_QSTR_derive),
MP_ROM_PTR(&mod_trezorcrypto_HDNode_derive_obj)},
{MP_ROM_QSTR(MP_QSTR_derive_path),
@@ -594,10 +600,13 @@ STATIC mp_obj_t mod_trezorcrypto_bip32_deserialize(mp_obj_t value, mp_obj_t vers
}
}
- mp_obj_HDNode_t *o = m_new_obj(mp_obj_HDNode_t);
+ // With a finaliser, like every other HDNode allocation here, so __del__ wipes
+ // it when it is collected rather than leaving it in the heap
+ mp_obj_HDNode_t *o = m_new_obj_with_finaliser(mp_obj_HDNode_t);
o->base.type = &mod_trezorcrypto_HDNode_type;
o->hdnode = hdnode;
o->fingerprint = fingerprint;
+ memzero(&hdnode, sizeof(hdnode));
return MP_OBJ_FROM_PTR(o);
}
### ports/stm32/boards/Passport/modules/flows/export_summary_flow.py
@@ -85,15 +85,14 @@ def generate_public_contents():
hard_sub, chain.serialize_public(node, addr_fmt)))
submaster = hard_sub
- # TODO: Add blank() back into trezor?
- # node.blank()
+ node.blank()
del node
# show the payment address
node = sv.derive_path(subpath, register=False)
yield ('%s => %s\n' % (subpath, chain.address(node, addr_fmt)))
- # TODO: Do we need to do this? node.blank()
+ node.blank()
del node
yield ('\n\n')
### ports/stm32/boards/Passport/modules/stash.py
@@ -34,8 +34,9 @@ def blank_object(item):
for i in range(ln):
buf[i] = 0
elif isinstance(item, trezorcrypto.bip32.HDNode):
- pass
- # item.blank() # node.blank() elsewhere
+ # Wipes the key material and the curve with it, so the node is inert
+ # afterwards. Every caller drops it immediately.
+ item.blank()
else:
raise TypeError(item)
@@ -107,6 +108,10 @@ def decode(secret, _bip39pw=''):
hd = trezorcrypto.bip32.from_seed(ms, 'secp256k1')
+ # The 64 byte master seed isn't returned to the caller, so nothing else
+ # will ever wipe it
+ blank_object(ms)
+
return 'words', seed_bits, hd
else:
### ports/stm32/boards/Passport/modules/tests/test_unit.py
@@ -44,6 +44,10 @@ def test_bip39_prefix_matching(test):
assert test('bip39_prefix_matching.py') == b'OK'
+def test_hdnode_blank(test):
+ assert test('hdnode_blank.py') == b'OK'
+
+
def test_psbt_multisig_approval(test):
assert test('psbt_multisig_approval.py') == b'OK'
### ports/stm32/boards/Passport/modules/tests/unit/hdnode_blank.py
@@ -0,0 +1,70 @@
+# SPDX-FileCopyrightText: © 2026 Foundation Devices, Inc. <hello@foundation.xyz>
+# SPDX-License-Identifier: GPL-3.0-or-later
+#
+# HDNode.blank() wipes key material on demand rather than waiting for the node to be
+# collected. It clears the whole node, curve pointer included, so a blanked node can
+# only be dropped; every caller does that immediately.
+
+import trezorcrypto
+
+from stash import blank_object
+
+SEED = bytes(range(64))
+XPUB_VERSION = 0x0488b21e
+ZEROS = bytes(32)
+
+
+def fresh_node():
+ node = trezorcrypto.bip32.from_seed(SEED, 'secp256k1')
+ node.derive(0x80000000)
+ return node
+
+
+def assert_blanked(node):
+ assert node.private_key() == ZEROS
+ assert node.private_key_ext() == ZEROS
+ assert node.chain_code() == ZEROS
+ assert node.depth() == 0
+ assert node.child_num() == 0
+ assert node.fingerprint() == 0
+
+
+node = fresh_node()
+
+# Everything that blank() clears is set to begin with, so the checks below mean
+# something.
+assert node.private_key() != ZEROS
+assert node.chain_code() != ZEROS
+assert node.depth() == 1
+assert node.child_num() == 0x80000000
+assert node.fingerprint() != 0
+
+node.blank()
+assert_blanked(node)
+
+# Blanking an already blank node is not an error.
+node.blank()
+assert_blanked(node)
+
+# stash.blank_object() reaches the same wipe. Until now its HDNode branch was a
+# no-op, so SensitiveValues.__exit__ left every registered node in the heap.
+node = fresh_node()
+assert node.private_key() != ZEROS
+blank_object(node)
+assert_blanked(node)
+
+# A node that carries no private key still has a chain code worth wiping.
+public_node = trezorcrypto.bip32.deserialize(fresh_node().serialize_public(XPUB_VERSION),
+ XPUB_VERSION, True)
+assert public_node.chain_code() != ZEROS
+public_node.blank()
+assert_blanked(public_node)
+
+# Anything blank_object() cannot wipe is still refused rather than passed over.
+try:
+ blank_object(1)
+ raise RuntimeError('expected TypeError')
+except TypeError:
+ pass
+
+return_value.write(b'OK')Why this scored 60/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.