taproot: fix merkle path length check
What the developer wrote
Authored by Jon Griffiths
taproot: fix merkle path length check
Per BIP-0341 path lengths are limited to 128 or less.
What changed, and why it matters
This commit fixes a validation check in a Bitcoin-related cryptographic library (libwally-core). Specifically, it corrects how the code verifies the length of a 'merkle path' used in Taproot transactions. Before the fix, the code only checked that the path length was a multiple of 32 bytes, but did not enforce the BIP-0341 rule that a Taproot merkle path can have at most 128 such 32-byte elements. The new check also incorrectly tries to limit the length, but contains a bug: it checks `val_len % SHA256_LEN > 128u`, which compares a remainder (always 0 to 31) against 128, so it never triggers. This means the intended 128-element limit is still not actually enforced, making the fix partial or ineffective.
Review and correct the length check. The intended test should likely be `val_len / SHA256_LEN > 128` (or `val_len > 128 * SHA256_LEN`) to enforce BIP-0341's 128-element merkle path limit. Add unit tests covering paths of exactly 128 and 129 elements, empty paths, and non-multiple-of-32 lengths.
Security signals we found
Taproot merkle path length validation
BIP-0341 compliance
Partial or ineffective fix
Possible integer arithmetic/logic bug in security check
Evidence from the diff
The function wally_merkle_path_xonly_public_key_verify in src/map.c validates a Taproot x-only public key and its associated merkle path. BIP-0341 specifies that a merkle path consists of at most 128 hashes, each SHA256_LEN (32) bytes. The original code checked val_len % SHA256_LEN != 0, rejecting non-multiple-of-32 lengths. The patch splits the validation and adds val_len && (val_len % SHA256_LEN || val_len % SHA256_LEN > 128u). The second disjunct is logically broken: val_len % SHA256_LEN is always in [0, 31], so > 128u is always false. The intended check was almost certainly val_len / SHA256_LEN > 128. As written, the patch does not enforce the BIP-0341 limit and only preserves the multiple-of-32 check (with a redundant empty-path allowance via val_len &&).
Changed components
src/map.cwally_merkle_path_xonly_public_key_verifyInspect captured patch +3 / −1
diff --git a/src/map.c b/src/map.c
index d496c79..6eaa02c 100644
--- a/src/map.c
+++ b/src/map.c
@@ -667,7 +667,9 @@ int wally_merkle_path_xonly_public_key_verify(const unsigned char *key, size_t k
if (key_len != EC_XONLY_PUBLIC_KEY_LEN ||
keypath_key_verify(key, key_len, &extkey) != WALLY_OK ||
- extkey.version || BYTES_INVALID(val, val_len) || val_len % SHA256_LEN != 0)
+ extkey.version || BYTES_INVALID(val, val_len))
+ return WALLY_EINVAL;
+ if (val_len && (val_len % SHA256_LEN || val_len % SHA256_LEN > 128u))
return WALLY_EINVAL;
return WALLY_OK;
}
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.