Skip to content

Commit 5e76f3f

Browse files
committed
fuzz: miniscript: higher sensitivity for max stack size limit under Tapscript
In order to exacerbate a mistake in the stack size tracking logic, sometimes pad the witness to make the script execute at the brink of the stack size limit. This way if the stack size is underestimated for a script it would immediately fail `VerifyScript`.
1 parent 6f529cb commit 5e76f3f

File tree

1 file changed

+34
-16
lines changed

1 file changed

+34
-16
lines changed

src/test/fuzz/miniscript.cpp

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ struct TestData {
9494
}
9595

9696
//! Get the (Schnorr or ECDSA, depending on context) signature for this pubkey.
97-
std::pair<std::vector<unsigned char>, bool>* GetSig(const MsCtx script_ctx, const Key& key) {
97+
const std::pair<std::vector<unsigned char>, bool>* GetSig(const MsCtx script_ctx, const Key& key) const {
9898
if (!miniscript::IsTapscript(script_ctx)) {
9999
const auto it = dummy_sigs.find(key);
100100
if (it == dummy_sigs.end()) return nullptr;
@@ -1059,9 +1059,10 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
10591059
assert(decoded->ToScript(PARSER_CTX) == script);
10601060
assert(decoded->GetType() == node->GetType());
10611061

1062-
const auto node_ops{node->GetOps()};
1063-
if (!IsTapscript(script_ctx) && provider.ConsumeBool() && node_ops && *node_ops < MAX_OPS_PER_SCRIPT
1064-
&& node->ScriptSize() < MAX_STANDARD_P2WSH_SCRIPT_SIZE) {
1062+
// Optionally pad the script or the witness in order to increase the sensitivity of the tests of
1063+
// the resources limits logic.
1064+
CScriptWitness witness_mal, witness_nonmal;
1065+
if (provider.ConsumeBool()) {
10651066
// Under P2WSH, optionally pad the script with OP_NOPs to max op the ops limit of the constructed script.
10661067
// This makes the script obviously not actually miniscript-compatible anymore, but the
10671068
// signatures constructed in this test don't commit to the script anyway, so the same
@@ -1071,10 +1072,25 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
10711072
// maximal.
10721073
// Do not pad more than what would cause MAX_STANDARD_P2WSH_SCRIPT_SIZE to be reached, however,
10731074
// as that also invalidates scripts.
1074-
int add = std::min<int>(
1075-
MAX_OPS_PER_SCRIPT - *node_ops,
1076-
MAX_STANDARD_P2WSH_SCRIPT_SIZE - node->ScriptSize());
1077-
for (int i = 0; i < add; ++i) script.push_back(OP_NOP);
1075+
const auto node_ops{node->GetOps()};
1076+
if (!IsTapscript(script_ctx) && node_ops && *node_ops < MAX_OPS_PER_SCRIPT
1077+
&& node->ScriptSize() < MAX_STANDARD_P2WSH_SCRIPT_SIZE) {
1078+
int add = std::min<int>(
1079+
MAX_OPS_PER_SCRIPT - *node_ops,
1080+
MAX_STANDARD_P2WSH_SCRIPT_SIZE - node->ScriptSize());
1081+
for (int i = 0; i < add; ++i) script.push_back(OP_NOP);
1082+
}
1083+
1084+
// Under Tapscript, optionally pad the stack up to the limit minus the calculated maximum execution stack
1085+
// size to assert a Miniscript would never add more elements to the stack during execution than anticipated.
1086+
const auto node_exec_ss{node->GetExecStackSize()};
1087+
if (miniscript::IsTapscript(script_ctx) && node_exec_ss && *node_exec_ss < MAX_STACK_SIZE) {
1088+
unsigned add{(unsigned)MAX_STACK_SIZE - *node_exec_ss};
1089+
witness_mal.stack.resize(add);
1090+
witness_nonmal.stack.resize(add);
1091+
script.reserve(add);
1092+
for (unsigned i = 0; i < add; ++i) script.push_back(OP_NIP);
1093+
}
10781094
}
10791095

10801096
SATISFIER_CTX.script_ctx = script_ctx;
@@ -1084,26 +1100,26 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
10841100
const CScript script_pubkey{ScriptPubKey(script_ctx, script, builder)};
10851101

10861102
// Run malleable satisfaction algorithm.
1087-
CScriptWitness witness_mal;
1088-
const bool mal_success = node->Satisfy(SATISFIER_CTX, witness_mal.stack, false) == miniscript::Availability::YES;
1089-
SatisfactionToWitness(script_ctx, witness_mal, script, builder);
1103+
std::vector<std::vector<unsigned char>> stack_mal;
1104+
const bool mal_success = node->Satisfy(SATISFIER_CTX, stack_mal, false) == miniscript::Availability::YES;
10901105

10911106
// Run non-malleable satisfaction algorithm.
1092-
CScriptWitness witness_nonmal;
1093-
const bool nonmal_success = node->Satisfy(SATISFIER_CTX, witness_nonmal.stack, true) == miniscript::Availability::YES;
1094-
SatisfactionToWitness(script_ctx, witness_nonmal, script, builder);
1107+
std::vector<std::vector<unsigned char>> stack_nonmal;
1108+
const bool nonmal_success = node->Satisfy(SATISFIER_CTX, stack_nonmal, true) == miniscript::Availability::YES;
10951109

10961110
if (nonmal_success) {
10971111
// Non-malleable satisfactions are bounded by the satisfaction size plus:
10981112
// - For P2WSH spends, the witness script
10991113
// - For Tapscript spends, both the witness script and the control block
11001114
const size_t max_stack_size{*node->GetStackSize() + 1 + miniscript::IsTapscript(script_ctx)};
1101-
assert(witness_nonmal.stack.size() <= max_stack_size);
1115+
assert(stack_nonmal.size() <= max_stack_size);
11021116
// If a non-malleable satisfaction exists, the malleable one must also exist, and be identical to it.
11031117
assert(mal_success);
1104-
assert(witness_nonmal.stack == witness_mal.stack);
1118+
assert(stack_nonmal == stack_mal);
11051119

11061120
// Test non-malleable satisfaction.
1121+
witness_nonmal.stack.insert(witness_nonmal.stack.end(), std::make_move_iterator(stack_nonmal.begin()), std::make_move_iterator(stack_nonmal.end()));
1122+
SatisfactionToWitness(script_ctx, witness_nonmal, script, builder);
11071123
ScriptError serror;
11081124
bool res = VerifyScript(DUMMY_SCRIPTSIG, script_pubkey, &witness_nonmal, STANDARD_SCRIPT_VERIFY_FLAGS, CHECKER_CTX, &serror);
11091125
// Non-malleable satisfactions are guaranteed to be valid if ValidSatisfactions().
@@ -1117,6 +1133,8 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
11171133

11181134
if (mal_success && (!nonmal_success || witness_mal.stack != witness_nonmal.stack)) {
11191135
// Test malleable satisfaction only if it's different from the non-malleable one.
1136+
witness_mal.stack.insert(witness_mal.stack.end(), std::make_move_iterator(stack_mal.begin()), std::make_move_iterator(stack_mal.end()));
1137+
SatisfactionToWitness(script_ctx, witness_mal, script, builder);
11201138
ScriptError serror;
11211139
bool res = VerifyScript(DUMMY_SCRIPTSIG, script_pubkey, &witness_mal, STANDARD_SCRIPT_VERIFY_FLAGS, CHECKER_CTX, &serror);
11221140
// Malleable satisfactions are not guaranteed to be valid under any conditions, but they can only

0 commit comments

Comments
 (0)