Fixing expected output array length for os_derive_bip32_with_seed_no_throw() SDK function
What the developer wrote
Authored by Ilya Artemov
Fixing expected output array length for os_derive_bip32_with_seed_no_throw() SDK function
What changed, and why it matters
This commit fixes a buffer-size mismatch in the Ledger Bitcoin app. A function that derives secret keys was being asked to write 64 bytes of output into a caller-supplied 32-byte buffer. The patch now provides a 64-byte temporary buffer and copies only the needed 32 bytes afterward, then securely wipes the temporary buffer. The original bug could have caused memory corruption or leaked key material, depending on how the SDK behaves when given an undersized buffer.
Review whether the undersized buffer could have been exploited on any shipped firmware/app version, and confirm the SDK's actual behavior when the output buffer is too small. If a crash or key leak was possible, treat this as a security fix and consider a coordinated disclosure or firmware update note. Otherwise, continue normal QA.
Security signals we found
Buffer size mismatch between SDK API contract (64 bytes) and caller buffer (32 bytes)
Potential stack buffer overflow or out-of-bounds write in key derivation path
Use of explicit_bzero to prevent temporary key material lingering on stack
SLIP21 derivation produces 64 bytes but only first 32 bytes are the symmetric key
Evidence from the diff
crypto_derive_symmetric_key() in src/crypto.c calls os_derive_bip32_with_seed_no_throw() with mode HDW_SLIP21. The SDK documentation/behavior for this function requires the output key array to be 64 bytes, but the caller was passing a 32-byte key buffer directly. The patch allocates a local 64-byte tmp_key, passes it to the SDK, copies the first 32 bytes (the usable SLIP21 key) to key on success, and uses explicit_bzero() to clear the temporary buffer. The return value handling is also refactored but semantically equivalent.
Changed components
src/crypto.ccrypto_derive_symmetric_key()os_derive_bip32_with_seed_no_throw() callerInspect captured patch +15 / −10
diff --git a/src/crypto.c b/src/crypto.c
index 1cf7ca5..8f4792a 100644
--- a/src/crypto.c
+++ b/src/crypto.c
@@ -289,18 +289,23 @@ bool crypto_derive_symmetric_key(const char *label, size_t label_len, uint8_t ke
memcpy(label_copy, label, label_len);
- if (os_derive_bip32_with_seed_no_throw(HDW_SLIP21,
- CX_CURVE_SECP256K1,
- (uint32_t *) label_copy,
- label_len,
- key,
- NULL,
- NULL,
- 0) != CX_OK) {
- return false;
+ // The SDK function below requires the output key array to be 64 bytes long
+ uint8_t tmp_key[64] = {0};
+ cx_err_t ret = os_derive_bip32_with_seed_no_throw(HDW_SLIP21,
+ CX_CURVE_SECP256K1,
+ (uint32_t *) label_copy,
+ label_len,
+ tmp_key,
+ NULL,
+ NULL,
+ 0);
+ if (ret == CX_OK) {
+ // Only the first 32 bytes are used for SLIP21
+ memcpy(key, tmp_key, 32);
}
+ explicit_bzero(tmp_key, sizeof(tmp_key));
- return true;
+ return ret == CX_OK;
}
int get_extended_pubkey_at_path(const uint32_t bip32_path[],
Why this scored 58/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.