Add regression test for unhardened derivation step parsing
What the developer wrote
Authored by Salvatore Ingala
Add regression test for unhardened derivation step parsing
What changed, and why it matters
This commit only adds a new regression test to the Bitcoin app's test suite. The test checks that wallet policy key expressions reject a specific boundary value (2147483648, the first 'hardened' child index) where only 'unhardened' values are allowed. The commit message says an older version of the code incorrectly accepted this boundary value, but the actual code fix is not present in this commit—only the test is added. So this is a test-only change that documents a previously fixed parsing bug.
Locate and review the earlier production-code commit that actually fixed the boundary check, since this commit only adds the regression test. Verify that the parser now rejects 0x80000000 and that no other hardened boundary values are accepted in unhardened contexts.
Security signals we found
Regression test for hardened/unhardened derivation index boundary parsing
Commit message states older version accepted 0x80000000 as unhardened
No production code change in this commit
Evidence from the diff
The diff adds test_parse_keyexpr_multipath_hardened_boundary() to unit-tests/test_wallet.c. It verifies that in a /<M;N>/* key expression, both M and N must be strictly below 0x80000000. The test asserts that 2147483647 (0x7fffffff) is accepted and that 2147483648 (0x80000000) is rejected in both positions. The commit message frames this as a regression test for an older parsing mistake where 0x80000000 was treated as valid unhardened. No parser implementation changes are included in the diff, so the security-relevant behavior was presumably corrected in an earlier commit.
Changed components
unit-tests/test_wallet.cwallet policy parser (tested indirectly; no source change in commit)Inspect captured patch +20 / −0
### unit-tests/test_wallet.c
@@ -368,6 +368,25 @@ static void test_parse_unsigned_decimal_overflow(void **state) {
assert_true(0 > parse_policy("wsh(older(5368709120))", out, sizeof(out)));
}
+// Regression test: in a /<M;N>/* key expression, M and N must be unhardened.
+// An older version incorrectly accepted 0x80000000 itself (the first hardened index) as a valid
+// unhardened value.
+static void test_parse_keyexpr_multipath_hardened_boundary(void **state) {
+ (void) state;
+
+ uint8_t out[MAX_WALLET_POLICY_MEMORY_SIZE];
+
+ // 0x7fffffff (2147483647) is the largest valid unhardened index: still accepted.
+ int res = parse_policy("pkh(@0/<2147483647;0>/*)", out, sizeof(out));
+ assert_true(res >= 0);
+ policy_node_with_key_t *node_1 = (policy_node_with_key_t *) out;
+ check_key_expr_plain(r_policy_node_keyexpr(&node_1->key), 0, 2147483647, 0);
+
+ // 0x80000000 (2147483648) is the first hardened index: must be rejected for both M and N.
+ assert_true(0 > parse_policy("pkh(@0/<2147483648;0>/*)", out, sizeof(out)));
+ assert_true(0 > parse_policy("pkh(@0/<0;2147483648>/*)", out, sizeof(out)));
+}
+
static void test_failures(void **state) {
(void) state;
@@ -1036,6 +1055,7 @@ int main() {
cmocka_unit_test(test_parse_policy_tr_musig_keypath),
cmocka_unit_test(test_get_policy_segwit_version),
cmocka_unit_test(test_parse_unsigned_decimal_overflow),
+ cmocka_unit_test(test_parse_keyexpr_multipath_hardened_boundary),
cmocka_unit_test(test_failures),
cmocka_unit_test(test_miniscript_types),
cmocka_unit_test(test_traverse_single_leaf),Why this scored 47/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.