Fixed taproot opcodes & buffer overflow

BlobMaster41 · BlobMaster41 · commit 023f52e85991 · 2025-03-24T03:21:29.000+04:00
Fixed taproot opcodes validation and buffer overflow
diff --git a/btcdeb.cpp b/btcdeb.cpp
@@ -242,9 +242,9 @@ int main(int argc, char* const* argv)
         }
     }
 
-    CScript script;
     if (script_str) {
-        if (instance.parse_script(script_str)) {
+        int witprogver = ((flags & SCRIPT_VERIFY_TAPROOT) != 0 ? 1 : 0);
+        if (instance.parse_script(witprogver, script_str)) {
             if (verbose) btc_logf("valid script\n");
         } else {
             fprintf(stderr, "invalid script\n");
@@ -300,6 +300,7 @@ int main(int argc, char* const* argv)
             p2sh_script = CScript(p2sh_script_payload.begin(), p2sh_script_payload.end());
         }
     }
+
     if (has_p2sh) {
         script_ptrs.push_back(&p2sh_script);
         script_headers.push_back("<<< P2SH script >>>");
@@ -316,19 +317,53 @@ int main(int argc, char* const* argv)
             script_lines[i++] = strdup(strprintf("#%04d %s", i, s).c_str());
         }
     }
+
     for (size_t siter = 0; siter < script_ptrs.size(); ++siter) {
         CScript* script = script_ptrs[siter];
         const std::string& header = script_headers[siter];
         if (header != "") script_lines[i++] = strdup(header.c_str());
         it = script->begin();
+
         while (script->GetOp(it, opcode, vchPushValue)) {
+            // log opcode and data
             char* pbuf = buf;
-            pbuf += snprintf(pbuf, 1024, "#%04d ", i);
+
+            // Write the line number
+            int written = snprintf(pbuf, sizeof(buf), "#%04d ", i);
+            if (written < 0 || written >= (int)sizeof(buf)) {
+                // Handle error or truncation
+                // For safety, we can bail out or clamp
+                written = (int)(sizeof(buf) - 1);
+            }
+            pbuf += written;
+
+            // Write the opcode
+            size_t remain = sizeof(buf) - (pbuf - buf);
             if (vchPushValue.size() > 0) {
-                snprintf(pbuf, 1024 + pbuf - buf, "%s", HexStr(std::vector<uint8_t>(vchPushValue.begin(), vchPushValue.end())).c_str());
+                written = snprintf(
+                    pbuf,
+                    remain,
+                    "%s",
+                    HexStr(std::vector<uint8_t>(vchPushValue.begin(), vchPushValue.end())).c_str()
+                );
             } else {
-                snprintf(pbuf, 1024 + pbuf - buf, "%s", GetOpName(opcode).c_str());
+                written = snprintf(
+                    pbuf,
+                    remain,
+                    "%s",
+                    GetOpName(opcode).c_str()
+                );
             }
+
+            // Handle error or truncation
+            if (written < 0 || (size_t)written >= remain) {
+                // Handle error or truncation
+                written = (int)(remain - 1);
+            }
+
+            pbuf += written;
+
+            // Write the buffer
             script_lines[i++] = strdup(buf);
         }
     }
@@ -349,6 +384,7 @@ int main(int argc, char* const* argv)
         }
 
         print_stack(env->stack, true);
+
         return 0;
     } else {
         kerl_set_history_file(".btcdeb_history");
@@ -371,6 +407,7 @@ int main(int argc, char* const* argv)
         if (env->curr_op_seq < count) {
             printf("%s\n", script_lines[env->curr_op_seq]);
         }
+
         kerl_run("btcdeb> ");
     }
 }
diff --git a/instance.cpp b/instance.cpp
@@ -93,7 +93,7 @@ bool Instance::parse_input_transaction(const char* txdata, int select_index) {
     return true;
 }
 
-bool Instance::parse_script(const char* script_str) {
+bool Instance::parse_script(int witprogver, const char* script_str) {
     std::vector<unsigned char> scriptData = Value(script_str).data_value();
     script = CScript(scriptData.begin(), scriptData.end());
     // for (const auto& keymap : COMPILER_CTX.keymap) {
@@ -110,12 +110,12 @@ bool Instance::parse_script(const char* script_str) {
     //     printf("miniscript failed to parse script; miniscript support disabled\n");
     //     msenv = nullptr;
     // }
-    return script.HasValidOps();
+    return witprogver != 0 || script.HasValidOps();
 }
 
-bool Instance::parse_script(const std::vector<uint8_t>& script_data) {
+bool Instance::parse_script(int witprogver, const std::vector<uint8_t>& script_data) {
     script = CScript(script_data.begin(), script_data.end());
-    return script.HasValidOps();
+    return witprogver != 0 || script.HasValidOps();
 }
 
 bool Instance::parse_pretend_valid_expr(const char* expr) {
@@ -545,7 +545,7 @@ bool Instance::configure_tx_txin() {
             }
         } else assert(!"should never get here; was a new witprogver added?");
 
-        if (parse_script(std::vector<uint8_t>(validation.begin(), validation.end()))) {
+        if (parse_script(witprogver, std::vector<uint8_t>(validation.begin(), validation.end()))) {
             btc_logf("valid script\n");
         } else {
             fprintf(stderr, "invalid script (witness stack last element)\n");
diff --git a/instance.h b/instance.h
@@ -68,8 +68,8 @@ class Instance {
     bool parse_transaction(const char* txdata, bool parse_amounts = false);
     bool parse_input_transaction(const char* txdata, int select_index = -1);
 
-    bool parse_script(const char* script_str);
-    bool parse_script(const std::vector<uint8_t>& script_data);
+    bool parse_script(int witprogver, const char* script_str);
+    bool parse_script(int witprogver, const std::vector<uint8_t>& script_data);
 
     void parse_stack_args(size_t argc, char* const* argv, size_t starting_index);
     void parse_stack_args(const std::vector<const char*> args);
diff --git a/script/script.cpp b/script/script.cpp
@@ -9,8 +9,15 @@
 
 #include <string>
 
+#include <tinyformat.h>
+
 std::string GetOpName(opcodetype opcode)
 {
+    if (opcode >= 0x01 && opcode <= 0x4b) {
+        // It's a push-data opcode for 'opcode' bytes
+        return strprintf("OP_PUSHBYTES_%d", (int)opcode);
+    }
+
     switch (opcode)
     {
     // push value
diff --git a/tap.cpp b/tap.cpp
@@ -265,7 +265,7 @@ int main(int argc, char* const* argv)
     for (size_t i = 0; i < script_count; ++i) {
         Item scriptData = Value(ca.l[2 + i]).data_value();
         CScript script = CScript(scriptData.begin(), scriptData.end());
-        if (!script.HasValidOps()) {
+        if (!is_tapscript && !script.HasValidOps()) {
             abort("invalid script #%zu: %s", i, HEXC(scriptData));
         }
         if (!quiet) {
diff --git a/test/signing.cpp b/test/signing.cpp
@@ -25,7 +25,7 @@ TEST_CASE("Segwit Multisig Signing", "[signing-segwit-multisig]") {
         Instance instance;
         instance.parse_transaction(TXAMT ":" TXHEX, true);
 
-        instance.parse_script(SCRIPT);
+        instance.parse_script(0, SCRIPT);
         // script should have 6 entries
         {
             size_t count = 0;
@@ -62,7 +62,7 @@ TEST_CASE("Segwit Multisig Signing", "[signing-segwit-multisig]") {
         #define STACK2X "304502207f874ef00f11dcc9a621acad9354f3fca1bf90c43878f607b7e2d358088487e7022052a01b47b8eef5e1c96a6affdc3dac46fdc11b60612464dc8c5921a852090d2701"
         Instance instance;
         instance.parse_transaction(TXAMT ":" TXHEX, true);
-        instance.parse_script(SCRIPT);
+        instance.parse_script(0, SCRIPT);
         const char* argv[] = {STACK1, STACK2X, STACK3};
         instance.parse_stack_args(3, (char* const*)argv, 0);
         instance.setup_environment();
@@ -86,7 +86,7 @@ TEST_CASE("Segwit Multisig Signing", "[signing-segwit-multisig]") {
         #define STACK3X "3045022100c56ab2abb17fdf565417228763bc9f2940a6465042fd62fbd9f4c7406345d7f702201cb1a56b45181f8347713627b325ec5df48fc1aee6bdaf937cbb804d7409b10c00"
         Instance instance;
         instance.parse_transaction(TXAMT ":" TXHEX, true);
-        instance.parse_script(SCRIPT);
+        instance.parse_script(0, SCRIPT);
         const char* argv[] = {STACK1, STACK2, STACK3X};
         instance.parse_stack_args(3, (char* const*)argv, 0);
         instance.setup_environment();
@@ -110,7 +110,7 @@ TEST_CASE("Segwit Multisig Signing", "[signing-segwit-multisig]") {
         #define TXAMTX "8.947025"
         Instance instance;
         instance.parse_transaction(TXAMTX ":" TXHEX, true);
-        instance.parse_script(SCRIPT);
+        instance.parse_script(0, SCRIPT);
         const char* argv[] = {STACK1, STACK2, STACK3};
         instance.parse_stack_args(3, (char* const*)argv, 0);
         instance.setup_environment();

Original file line number	Diff line number	Diff line change
`@@ -9,8 +9,15 @@`
`9`	`9`
`10`	`10`	`#include <string>`
`11`	`11`
	`12`	`+#include <tinyformat.h>`
	`13`	`+`
`12`	`14`	`std::string GetOpName(opcodetype opcode)`
`13`	`15`	`{`
	`16`	`+ if (opcode >= 0x01 && opcode <= 0x4b) {`
	`17`	`+ // It's a push-data opcode for 'opcode' bytes`
	`18`	`+ return strprintf("OP_PUSHBYTES_%d", (int)opcode);`
	`19`	`+ }`
	`20`	`+`
`14`	`21`	`switch (opcode)`
`15`	`22`	`{`
`16`	`23`	`// push value`
Original file line number	Diff line number	Diff line change
`@@ -265,7 +265,7 @@ int main(int argc, char* const* argv)`
`265`	`265`	`for (size_t i = 0; i < script_count; ++i) {`
`266`	`266`	`Item scriptData = Value(ca.l[2 + i]).data_value();`
`267`	`267`	`CScript script = CScript(scriptData.begin(), scriptData.end());`
`268`		`- if (!script.HasValidOps()) {`
	`268`	`+ if (!is_tapscript && !script.HasValidOps()) {`
`269`	`269`	`abort("invalid script #%zu: %s", i, HEXC(scriptData));`
`270`	`270`	`}`
`271`	`271`	`if (!quiet) {`