keychain: fix error handling for cached green service keys
What the developer wrote
Authored by Jon Griffiths
keychain: fix error handling for cached green service keys
What changed, and why it matters
This commit hardens error handling in a hardware wallet's key derivation code by wrapping two function calls with an assertion macro. Previously, if these functions failed silently, the device could continue with invalid or uninitialized cryptographic keys. Now the device will halt if those functions fail, preventing use of bad keys. The actual security impact depends on whether those failures were reachable in practice.
Treat as a defensive security hardening fix. Review whether the underlying functions can fail under realistic conditions (e.g., malformed service keys, memory corruption, or supply-chain tampering) and ensure JADE_ASSERT behavior is appropriate for a hardware wallet. No immediate exploit is evident from the diff alone.
Security signals we found
Unchecked return values from cryptographic key derivation functions
Potential use of invalid or uninitialized cached service keys
Addition of fatal assertions on failure paths
Change described by author as 'fix error handling'
Evidence from the diff
The patch adds JADE_ASSERT() around wallet_get_gaservice_root_key() and wallet_calculate_gaservice_path() calls in main/keychain.c. These functions return bool success/failure; previously their return values were discarded. The change converts silent failures into fatal assertions, ensuring the device does not cache or use potentially invalid Green service root keys or derivation paths. This is a defensive hardening fix rather than a patch for a demonstrated exploit.
Changed components
main/keychain.ckeychain_cached_service()keychain_derive_from_seed()Green service key derivation/cachingInspect captured patch +3 / −3
diff --git a/main/keychain.c b/main/keychain.c
index 87e03c9..a99a51b 100644
--- a/main/keychain.c
+++ b/main/keychain.c
@@ -257,8 +257,8 @@ const struct ext_key* keychain_cached_service(const struct ext_key* const servic
// Recompute cached values if service mismatch
if (service != keychain_data->cached_service) {
- wallet_get_gaservice_root_key(service, false, &keychain_data->cached_gaservice_main_root);
- wallet_get_gaservice_root_key(service, true, &keychain_data->cached_gaservice_subact_root);
+ JADE_ASSERT(wallet_get_gaservice_root_key(service, false, &keychain_data->cached_gaservice_main_root));
+ JADE_ASSERT(wallet_get_gaservice_root_key(service, true, &keychain_data->cached_gaservice_subact_root));
keychain_data->cached_service = service;
}
@@ -307,7 +307,7 @@ void keychain_derive_from_seed(const uint8_t* seed, const size_t seed_len, keych
wally_asset_blinding_key_from_seed(seed, seed_len, keydata->master_unblinding_key, HMAC_SHA512_LEN));
// Compute and cache the path the GA server will use to sign
- wallet_calculate_gaservice_path(&keydata->xpriv, keydata->gaservice_path, GASERVICE_PATH_LEN);
+ JADE_ASSERT(wallet_calculate_gaservice_path(&keydata->xpriv, keydata->gaservice_path, GASERVICE_PATH_LEN));
// Ensure cached green-multisig service path roots are unset
keydata->cached_service = NULL;
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.