Increase MAX_POLICY_KEY_INFO_LEN to the actual maximum; nits from PR review
What the developer wrote
Authored by Salvatore Ingala
Increase MAX_POLICY_KEY_INFO_LEN to the actual maximum; nits from PR review
The value of MAX_POLICY_KEY_INFO_LEN did not quite reflect the
actual maximum. Unlikely to be hit in practice, because derivation
steps used in practice are small - but this fixes it, with some
increase in memory usage during wallet registration.
Other nits from PR review.
What changed, and why it matters
This commit fixes a size limit in the Ledger Bitcoin app that was too small. The app uses this limit when registering Bitcoin wallet policies (descriptions of how to spend coins). The old limit underestimated how long a key description can be when many derivation steps are used. If a real wallet policy exceeded the old limit, the app could reject valid wallets or, in the worst case, mishandle memory. The commit also removes an unused header and updates related comments.
Treat this as a hardening/correctness fix. Review callers that use MAX_POLICY_KEY_INFO_LEN and MAX_WALLET_POLICY_SERIALIZED_LENGTH_* to ensure they now allocate sufficient memory and do not retain any hard-coded smaller assumptions. No immediate incident response is indicated, but users should update to a build containing this commit if they register wallets with deep or non-standard derivation paths.
Security signals we found
Buffer/limit size correction for key origin info
Removal of unused ledger_assert.h include
Comment-only updates to serialized wallet policy length bounds
Memory usage increase during wallet registration explicitly noted
Evidence from the diff
The change increases MAX_POLICY_KEY_INFO_LEN_V1 and MAX_POLICY_KEY_INFO_LEN_V2 from 46-byte key-origin allowance to 106 bytes, reflecting the documented formula (2 + 8 + 812). MAX_SERIALIZED_PUBKEY_LENGTH and the optional ‘/*’ suffix remain unchanged, so the total maximum key-info string grows from 162 to 222 bytes for V1 and similarly for V2. Derived serialized wallet policy length limits are updated in comments from 92/100 to 140/148 bytes, though the actual macros already used the larger constants. The commit removes the unused ledger_assert.h include. The patch is defensive and corrects an undersized buffer/limit; it does not by itself introduce a vulnerability, but the prior undersizing could have caused buffer truncation, parsing failures, or memory corruption if a long key origin was supplied.
Changed components
src/common/wallet.hWallet policy key info length limits (V1 and V2)Wallet registration memory allocation pathsInspect captured patch +5 / −6
### src/common/wallet.h
@@ -6,7 +6,6 @@
/* SDK headers */
#include "bip32.h"
#include "buffer.h"
-#include "ledger_assert.h"
/* Local headers */
#include "constants.h"
@@ -30,14 +29,14 @@
#define WALLET_POLICY_VERSION_V2 2 // the current full version
// The string describing a pubkey can contain:
-// - (optional) the key origin info, which we limit to 46 bytes (2 + 8 + 3*12 = 46 bytes)
+// - (optional) the key origin info, which we limit to 46 bytes (2 + 8 + 8*12 = 106 bytes)
// - the xpub itself (up to 113 characters)
// - optional, the "/**" suffix.
// Therefore, the total length of the key info string is at most 162 bytes.
-#define MAX_POLICY_KEY_INFO_LEN_V1 (46 + MAX_SERIALIZED_PUBKEY_LENGTH + 3)
+#define MAX_POLICY_KEY_INFO_LEN_V1 (106 + MAX_SERIALIZED_PUBKEY_LENGTH + 3)
// In V1, there is no "/**" suffix, as that is no longer part of the key
-#define MAX_POLICY_KEY_INFO_LEN_V2 (46 + MAX_SERIALIZED_PUBKEY_LENGTH)
+#define MAX_POLICY_KEY_INFO_LEN_V2 (106 + MAX_SERIALIZED_PUBKEY_LENGTH)
#define MAX_POLICY_KEY_INFO_LEN MAX(MAX_POLICY_KEY_INFO_LEN_V1, MAX_POLICY_KEY_INFO_LEN_V2)
@@ -85,7 +84,7 @@
// This limit is extremely unlikely to be hit in practice.
#define MAX_N_IN_THRESH 24
-// at most 92 bytes
+// at most 140 bytes
// wallet type (1 byte)
// name length (1 byte)
// name (max MAX_WALLET_NAME_LENGTH bytes)
@@ -96,7 +95,7 @@
#define MAX_WALLET_POLICY_SERIALIZED_LENGTH_V1 \
(1 + 1 + MAX_WALLET_NAME_LENGTH + 1 + MAX_DESCRIPTOR_TEMPLATE_LENGTH_V1 + 1 + 32)
-// at most 100 bytes
+// at most 148 bytes
// wallet type (1 byte)
// name length (1 byte)
// name (max MAX_WALLET_NAME_LENGTH bytes)Why this scored 37/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.