descriptor: early-reject descriptors with too many signers
What the developer wrote
Authored by Jon Griffiths
descriptor: early-reject descriptors with too many signers
Reported-by: DARKNAVY <X@DarkNavyOrg>
What changed, and why it matters
This update fixes a boundary check in how Blockstream Jade handles wallet descriptors (the instructions that say how many keys are needed to spend funds). Previously, the device accepted descriptors with one more signer than its internal limit, which could cause memory corruption or unexpected behavior. The fix rejects oversized descriptors earlier and adds tests to confirm the boundary.
Treat this as a security-hardening fix and include it in the next firmware release. Review whether the off-by-one could have been reachable from host software and assess if any additional bounds checks around descriptor parsing are needed.
Security signals we found
off-by-one boundary check corrected
explicit upper-bound validation added on untrusted CBOR input
assertion relaxed to allow the documented maximum
new negative test cases for empty and oversized signer maps
Evidence from the diff
The commit changes two checks in main/process/register_descriptor.c. First, it replaces a strict-less-than assertion with less-than-or-equal, so num_values equal to MAX_ALLOWED_SIGNERS is accepted. Second, it adds an explicit upper-bound check (num_map_items > MAX_ALLOWED_SIGNERS) during CBOR parameter parsing so descriptors with too many signers are rejected before processing. A test case is added for both empty and 16-signer descriptor maps.
Changed components
main/process/register_descriptor.ctest_jade.pyInspect captured patch +10 / −2
diff --git a/main/process/register_descriptor.c b/main/process/register_descriptor.c
index eb7c837..de67dba 100644
--- a/main/process/register_descriptor.c
+++ b/main/process/register_descriptor.c
@@ -30,7 +30,7 @@ static int register_descriptor(
JADE_INIT_OUT_PPTR(errmsg);
JADE_ASSERT(descriptor->script_len < sizeof(descriptor->script));
- JADE_ASSERT(descriptor->num_values < MAX_ALLOWED_SIGNERS);
+ JADE_ASSERT(descriptor->num_values <= MAX_ALLOWED_SIGNERS);
// Not valid for liquid wallets atm
if (network_is_liquid(network_id)) {
@@ -162,7 +162,8 @@ static bool get_data_values(const char* field, const CborValue* value, descripto
}
size_t num_map_items = 0;
- if (cbor_value_get_map_length(&result, &num_map_items) != CborNoError || !num_map_items) {
+ if (cbor_value_get_map_length(&result, &num_map_items) != CborNoError || !num_map_items
+ || num_map_items > MAX_ALLOWED_SIGNERS) {
return false;
}
diff --git a/test_jade.py b/test_jade.py
index dd4b599..1a7e9e3 100644
--- a/test_jade.py
+++ b/test_jade.py
@@ -1271,6 +1271,13 @@ HmWPvgD3hiTnD5KZuMkxSUsgGraZ9vavB5JSA3F9s5E4cXuCte5rvBs5N4DjfxYssQk1L82Bq4FE"
(('baddescr17', 'register_descriptor',
{'network': 'liquid', 'descriptor_name': 'isgood', 'descriptor': DESCRIPTOR,
'datavalues': {'@0': DESCR_SIGNER}}), 'not supported on liquid'),
+ (('baddescr18', 'register_descriptor',
+ {'network': 'testnet', 'descriptor_name': 'too_few', 'descriptor': 'test',
+ 'datavalues': {}}), 'Failed to extract valid parameter values'),
+ (('baddescr19', 'register_descriptor',
+ {'network': 'testnet', 'descriptor_name': 'too_many', 'descriptor': 'test',
+ 'datavalues': {"%15d" % i: "x" * 159 for i in range(16)}}),
+ 'Failed to extract valid parameter values'),
(('badrecvaddr1', 'get_receive_address'), 'Expecting parameters map'),
(('badrecvaddr2', 'get_receive_address',
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.