Limit legacy multisig to 15 keys maximum, per consensus limits
What the developer wrote
Authored by Salvatore Ingala
Limit legacy multisig to 15 keys maximum, per consensus limits
The redeem script would otherwise be bigger than 520 bytes, which
is the consensus limit; therefore, such scripts are unspendable
and funds sent to such addresses would be lost.
Therefore, we prevent registration altogether.
What changed, and why it matters
This commit fixes a bug in Ledger's Bitcoin app where users could register a wallet policy for a legacy P2SH multisig with up to 16 public keys. Such a redeem script would be larger than the 520-byte Bitcoin consensus limit, making the coins unspendable and effectively lost. The patch caps legacy P2SH multisig at 15 keys and rejects anything larger during wallet registration. It does not affect SegWit P2WSH or nested sh(wsh(...)) wallets, which have different limits.
Treat as a security-hardening fix and include in the next firmware/app release. No immediate incident response is required because the issue is self-DoS/funds-loss rather than theft or remote code execution. Users who already registered a 16-key legacy P2SH policy should be advised that funds sent to those addresses are unspendable on-chain and cannot be recovered by the device.
Security signals we found
Funds-loss prevention: blocks registration of wallet policies that would produce unspendable outputs
Consensus-rule enforcement: aligns with Bitcoin's 520-byte script push limit
Input validation hardening: adds explicit bounds check during policy parsing
Static assertion: compile-time guarantee that the constant satisfies the size limit
Evidence from the diff
The change adds a runtime check in parse_script() (src/common/wallet.c) and a compile-time static assertion in src/common/wallet.h. When parsing a multi/sortedmulti expression inside a legacy P2SH context (CONTEXT_WITHIN_SH set, CONTEXT_WITHIN_WSH clear), node->n is now compared against the new MAX_PUBKEYS_PER_P2SH_MULTISIG constant (15). If exceeded, parsing fails with ‘Too many keys for multisig in legacy P2SH’. The static_assert verifies that 3 + 34*15 <= 520, matching Bitcoin’s MAX_SCRIPT_ELEMENT_SIZE consensus rule. Unit tests confirm 16-key sh(multi(…)) and sh(sortedmulti(…)) are rejected, 15-key variants are accepted, and 16-key wsh(…) / sh(wsh(…)) remain accepted.
Changed components
src/common/wallet.c parse_script()src/common/wallet.h policy parsing constantsunit-tests/test_wallet.c regression testsInspect captured patch +40 / −0
### src/common/wallet.c
@@ -1796,6 +1796,13 @@ static int parse_script(buffer_t *in_buf,
return WITH_ERROR(-1, "Invalid k and/or n");
}
+ // In legacy P2SH, the redeem script must not exceed the 520-byte consensus limit
+ if ((context_flags & CONTEXT_WITHIN_SH) != 0 &&
+ (context_flags & CONTEXT_WITHIN_WSH) == 0 &&
+ node->n > MAX_PUBKEYS_PER_P2SH_MULTISIG) {
+ return WITH_ERROR(-1, "Too many keys for multisig in legacy P2SH");
+ }
+
if (token == TOKEN_SORTEDMULTI || token == TOKEN_SORTEDMULTI_A) {
node->base.flags.is_miniscript = 0;
} else if (token == TOKEN_MULTI) {
### src/common/wallet.h
@@ -18,6 +18,12 @@
// bitcoin-core supports up to 20, but we limit to 16 as bigger pushes require special handling.
#define MAX_PUBKEYS_PER_MULTISIG 16
+// The maximum number of keys for multi/sortedmulti in legacy P2SH: the redeem script
+// (3 + 34*n bytes) must not exceed the 520-byte consensus limit for stack elements.
+#define MAX_PUBKEYS_PER_P2SH_MULTISIG 15
+_Static_assert(3 + 34 * MAX_PUBKEYS_PER_P2SH_MULTISIG <= 520,
+ "P2SH multisig redeem script exceeds 520 bytes");
+
// The maximum number of keys supported in a musig() key expression
// It is basically unlimited in theory, but we need to set a practical limit.
// The implementation of MuSig2 requires quite a few large arrays (for example, the pubnonces are
### unit-tests/test_wallet.c
@@ -523,6 +523,33 @@ static void test_failures(void **state) {
"@17/**))",
out,
sizeof(out)));
+
+ // legacy P2SH multisig is limited to 15 keys, as the redeem script can't exceed 520 bytes
+ assert_true(0 > parse_policy("sh(multi(2,@0/**,@1/**,@2/**,@3/**,@4/**,@5/**,@6/**,@7/**,"
+ "@8/**,@9/**,@10/**,@11/**,@12/**,@13/**,@14/**,@15/**))",
+ out,
+ sizeof(out)));
+ assert_true(0 > parse_policy("sh(sortedmulti(2,@0/**,@1/**,@2/**,@3/**,@4/**,@5/**,@6/**,@7/**,"
+ "@8/**,@9/**,@10/**,@11/**,@12/**,@13/**,@14/**,@15/**))",
+ out,
+ sizeof(out)));
+ assert_true(0 <= parse_policy("sh(multi(2,@0/**,@1/**,@2/**,@3/**,@4/**,@5/**,@6/**,@7/**,"
+ "@8/**,@9/**,@10/**,@11/**,@12/**,@13/**,@14/**))",
+ out,
+ sizeof(out)));
+ assert_true(0 <= parse_policy("sh(sortedmulti(2,@0/**,@1/**,@2/**,@3/**,@4/**,@5/**,@6/**,"
+ "@7/**,@8/**,@9/**,@10/**,@11/**,@12/**,@13/**,@14/**))",
+ out,
+ sizeof(out)));
+ // no such limit for segwit
+ assert_true(0 <= parse_policy("wsh(multi(2,@0/**,@1/**,@2/**,@3/**,@4/**,@5/**,@6/**,@7/**,"
+ "@8/**,@9/**,@10/**,@11/**,@12/**,@13/**,@14/**,@15/**))",
+ out,
+ sizeof(out)));
+ assert_true(0 <= parse_policy("sh(wsh(sortedmulti(2,@0/**,@1/**,@2/**,@3/**,@4/**,@5/**,@6/**,"
+ "@7/**,@8/**,@9/**,@10/**,@11/**,@12/**,@13/**,@14/**,@15/**)))",
+ out,
+ sizeof(out)));
}
enum TestMode {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.