descriptor: require type B for the top level miniscript in sh()/wsh()
What the developer wrote
Authored by odudex
descriptor: require type B for the top level miniscript in sh()/wsh()
What changed, and why it matters
This commit tightens validation for Bitcoin descriptors that wrap a miniscript inside sh() (pay-to-script-hash) or wsh() (pay-to-witness-script-hash). Previously, the library accepted any miniscript fragment as the top-level child, even fragments that are not valid standalone scripts (for example, a fragment that only pushes a public key or verifies a signature but leaves a value on the stack). The change now rejects those malformed descriptors before they can be used to create addresses or transactions. The risk is that an attacker or buggy wallet could trick a user into accepting a descriptor whose script does not behave as expected, potentially making funds unspendable or enabling unexpected spending conditions.
Treat this as a security-relevant correctness fix. Upgrade to a version containing this commit, especially if your application parses or accepts user-supplied descriptors. Review any previously accepted sh()/wsh() descriptors to confirm their top-level miniscript is type B, and avoid using descriptors parsed before this fix for address generation or transaction signing without re-validation.
Security signals we found
Input-validation hardening for descriptor parsing
Rejection of non-top-level miniscript fragments in script wrappers
New negative test vectors for malformed descriptors
Potential for funds to become unspendable or for unexpected script semantics if malformed descriptors were accepted
Evidence from the diff
The patch adds child_is_valid_script() and calls it from verify_sh() and verify_wsh(). It requires that any miniscript child of sh()/wsh() have the miniscript type property TYPE_B, which denotes a complete, valid top-level script. Fragments of type V (verify, leaves nothing on stack), K (key, pushes a key), or W (wrapped, pushes 0/1) are now rejected. Descriptor-only children such as sortedmulti, which have no miniscript type, remain allowed. New negative tests cover wsh(v:pk()), wsh(pk_k()), wsh(a:pk()), sh(v:pk()), and sh(wsh(v:pk())).
Changed components
src/descriptor.csrc/ctest/test_descriptor.cDescriptor parsing for sh() and wsh() wrappersInspect captured patch +33 / −2
diff --git a/src/ctest/test_descriptor.c b/src/ctest/test_descriptor.c
index 93cb744..b2b0865 100644
--- a/src/ctest/test_descriptor.c
+++ b/src/ctest/test_descriptor.c
@@ -1247,6 +1247,26 @@ static const struct descriptor_test {
"descriptor - wsh - multi-child",
"wsh(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556,03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556)",
WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "", VARS_STD
+ },{
+ "descriptor - wsh - non-B top level miniscript (V)",
+ "wsh(v:pk(key_1))",
+ WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "", VARS_STD
+ },{
+ "descriptor - wsh - non-B top level miniscript (K)",
+ "wsh(pk_k(key_1))",
+ WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "", VARS_STD
+ },{
+ "descriptor - wsh - non-B top level miniscript (W)",
+ "wsh(a:pk(key_1))",
+ WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "", VARS_STD
+ },{
+ "descriptor - sh - non-B top level miniscript",
+ "sh(v:pk(key_1))",
+ WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "", VARS_STD
+ },{
+ "descriptor - sh-wsh - non-B top level miniscript",
+ "sh(wsh(v:pk(key_1)))",
+ WALLY_NETWORK_BITCOIN_MAINNET, 0, 0, 0, NULL, 0, NULL, "", VARS_STD
},{
"descriptor - pk - non-key child",
"pk(1)",
diff --git a/src/descriptor.c b/src/descriptor.c
index fcddd79..ff9e282 100644
--- a/src/descriptor.c
+++ b/src/descriptor.c
@@ -644,10 +644,20 @@ int wally_descriptor_free(ms_ctx *ctx)
return WALLY_OK;
}
+static bool child_is_valid_script(const ms_node *node)
+{
+ /* Miniscript children must be top-level, i.e. type B. Descriptor-only
+ * children (e.g. sortedmulti) have no miniscript type and are not checked.
+ */
+ return !(node->child->kind & KIND_MINISCRIPT) ||
+ (node->child->type_properties & TYPE_B);
+}
+
static int verify_sh(ms_ctx *ctx, ms_node *node)
{
(void)ctx;
- if (!node_is_top(node) || !node->child->builtin)
+ if (!node_is_top(node) || !node->child->builtin ||
+ !child_is_valid_script(node))
return WALLY_EINVAL;
node->type_properties = node->child->type_properties;
@@ -660,7 +670,8 @@ static int verify_wsh(ms_ctx *ctx, ms_node *node)
if (node->parent && node->parent->kind != KIND_DESCRIPTOR_SH &&
node->parent->kind != KIND_DESCRIPTOR_CT)
return WALLY_EINVAL;
- if (!node->child->builtin || node_has_uncompressed_key(ctx, node))
+ if (!node->child->builtin || !child_is_valid_script(node) ||
+ node_has_uncompressed_key(ctx, node))
return WALLY_EINVAL;
node->type_properties = node->child->type_properties;
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.