What the developer wrote
Authored by Salvatore Ingala
Validate boundaries before assigning
What changed, and why it matters
This commit fixes a boundary-check ordering bug in the Ledger Bitcoin app's wallet policy parser. Previously, the code stored an untrusted length value into a data structure before checking whether that length was too large. The fix moves the storage to after the validation check. This prevents a malformed wallet policy from causing the app to record an oversized length that could later lead to memory corruption or incorrect parsing.
Treat this as a security-relevant hardening fix and include it in the next firmware/app release. Review nearby parsing code for similar validation-ordering issues, especially where untrusted length fields are stored into persistent or output structures before being checked. No immediate public incident response appears required unless exploitation is demonstrated.
Security signals we found
Out-of-bounds length stored before validation
Wallet policy header parsing on a security device
Potential memory corruption or information leak from malformed policy
Fix reorders assignment to occur after boundary checks
Evidence from the diff
In read_wallet_policy_header() in src/common/wallet.c, the original code assigned descriptor_template_len to header->descriptor_template_len immediately after reading the varint, before validating it against MAX_DESCRIPTOR_TEMPLATE_LENGTH_V1 or MAX_DESCRIPTOR_TEMPLATE_LENGTH_V2. The patch moves that assignment to after both version-specific length checks and the actual buffer read. This closes a window where header->descriptor_template_len could hold an attacker-controlled, out-of-bounds value. The function then uses the local descriptor_template_len variable for the read, so the behavior is also more consistent. The bug is a classic TOCTOU-style ordering issue between parsing and validation.
Changed components
src/common/wallet.cread_wallet_policy_header()Ledger Bitcoin app wallet policy parsingInspect captured patch +3 / −2
diff --git a/src/common/wallet.c b/src/common/wallet.c
index be6cad2..c8d005f 100644
--- a/src/common/wallet.c
+++ b/src/common/wallet.c
@@ -132,7 +132,6 @@ int read_wallet_policy_header(buffer_t *buffer, policy_map_wallet_header_t *head
if (!buffer_read_varint(buffer, &descriptor_template_len)) {
return WITH_ERROR(-1, "Invalid wallet policy header");
}
- header->descriptor_template_len = (uint16_t) descriptor_template_len;
if (header->version == WALLET_POLICY_VERSION_V1) {
if (descriptor_template_len > MAX_DESCRIPTOR_TEMPLATE_LENGTH_V1) {
@@ -140,7 +139,7 @@ int read_wallet_policy_header(buffer_t *buffer, policy_map_wallet_header_t *head
}
if (!buffer_read_bytes(buffer,
(uint8_t *) header->descriptor_template,
- header->descriptor_template_len)) {
+ descriptor_template_len)) {
return WITH_ERROR(-1, "Invalid wallet policy header");
}
} else { // WALLET_POLICY_VERSION_V2
@@ -153,6 +152,8 @@ int read_wallet_policy_header(buffer_t *buffer, policy_map_wallet_header_t *head
}
}
+ header->descriptor_template_len = (uint16_t) descriptor_template_len;
+
uint64_t n_keys;
if (!buffer_read_varint(buffer, &n_keys) || n_keys > 252) {
return WITH_ERROR(-1, "Invalid wallet policy header");
Why this scored 59/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.