From fa53d3d8266ad0257315d07b71b4f8a711134622 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 11 Oct 2021 14:09:11 +0200 Subject: [PATCH 1/3] test: Check that bitcoin-tx accepts whitespace around sequence id and multisig numbers --- test/util/data/bitcoin-util-test.json | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 20684f0f76c..7228885c9db 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -523,6 +523,14 @@ "output_cmp": "txcreatedata_seq0.hex", "description": "Creates a new transaction with one input with sequence number and one address output" }, + { "exec": "./bitcoin-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0: 4294967293 ", + "outaddr=0.18:13tuJJDR2RgArmgfv6JScSdreahzgc4T6o"], + "output_cmp": "txcreatedata_seq0.hex", + "description": "Creates a new transaction with one input with sequence number (+whitespace) and one address output" + }, { "exec": "./bitcoin-tx", "args": ["-json", @@ -553,9 +561,9 @@ "description": "Creates a new transaction with a single 2-of-3 multisig output" }, { "exec": "./bitcoin-tx", - "args": ["-json", "-create", "outmultisig=1:2:3:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397:021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d:02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485", "nversion=1"], + "args": ["-json", "-create", "outmultisig=1: 2 : 3 :02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397:021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d:02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485", "nversion=1"], "output_cmp": "txcreatemultisig1.json", - "description": "Creates a new transaction with a single 2-of-3 multisig output (output in json)" + "description": "Creates a new transaction with a single 2-of-3 multisig output (with whitespace, output in json)" }, { "exec": "./bitcoin-tx", "args": ["-create", "outmultisig=1:2:3:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397:021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d:02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485:S", "nversion=1"], From fafab8ea5e6ed6b87fac57a5cd16a8135236cdd6 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 11 Oct 2021 14:21:03 +0200 Subject: [PATCH 2/3] bitcoin-tx: Reject non-integral and out of range sequence ids --- src/bitcoin-tx.cpp | 15 +++++++++++++-- test/lint/lint-locale-dependence.sh | 1 - test/util/data/bitcoin-util-test.json | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index fc3bc6aa713..4a9818d4ec1 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -235,6 +235,16 @@ static void MutateTxRBFOptIn(CMutableTransaction& tx, const std::string& strInId } } +template +static T TrimAndParse(const std::string& int_str, const std::string& err) +{ + const auto parsed{ToIntegral(TrimString(int_str))}; + if (!parsed.has_value()) { + throw std::runtime_error(err + " '" + int_str + "'"); + } + return parsed.value(); +} + static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInput) { std::vector vStrInputParts; @@ -261,8 +271,9 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu // extract the optional sequence number uint32_t nSequenceIn = CTxIn::SEQUENCE_FINAL; - if (vStrInputParts.size() > 2) - nSequenceIn = std::stoul(vStrInputParts[2]); + if (vStrInputParts.size() > 2) { + nSequenceIn = TrimAndParse(vStrInputParts.at(2), "invalid TX sequence id"); + } // append to transaction input list CTxIn txin(txid, vout, CScript(), nSequenceIn); diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index f82d82f8905..6ccd8638d33 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -41,7 +41,6 @@ export LC_ALL=C # independent ToIntegral(...) or the ParseInt*() functions. # TODO: Reduce KNOWN_VIOLATIONS by replacing uses of locale dependent snprintf with strprintf. KNOWN_VIOLATIONS=( - "src/bitcoin-tx.cpp.*stoul" "src/dbwrapper.cpp:.*vsnprintf" "src/rest.cpp:.*strtol" "src/test/dbwrapper_tests.cpp:.*snprintf" diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 7228885c9db..36ebda774b4 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -515,6 +515,30 @@ "output_cmp": "txcreatedata2.json", "description": "Creates a new transaction with one input, one address output and one data (zero value) output (output in json)" }, + { "exec": "./bitcoin-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0:11aa"], + "return_code": 1, + "error_txt": "error: invalid TX sequence id '11aa'", + "description": "Try to parse a sequence number outside the allowed range" + }, + { "exec": "./bitcoin-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0:-1"], + "return_code": 1, + "error_txt": "error: invalid TX sequence id '-1'", + "description": "Try to parse a sequence number outside the allowed range" + }, + { "exec": "./bitcoin-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0:4294967296"], + "return_code": 1, + "error_txt": "error: invalid TX sequence id '4294967296'", + "description": "Try to parse a sequence number outside the allowed range" + }, { "exec": "./bitcoin-tx", "args": ["-create", From fa6f29de516c7af5206b91b59ada466032329250 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 11 Oct 2021 14:33:38 +0200 Subject: [PATCH 3/3] bitcoin-tx: Reject non-integral and out of range multisig numbers --- src/bitcoin-tx.cpp | 4 ++-- test/util/data/bitcoin-util-test.json | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 4a9818d4ec1..eb97cfc6f6e 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -363,10 +363,10 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s CAmount value = ExtractAndValidateValue(vStrInputParts[0]); // Extract REQUIRED - uint32_t required = stoul(vStrInputParts[1]); + const uint32_t required{TrimAndParse(vStrInputParts.at(1), "invalid multisig required number")}; // Extract NUMKEYS - uint32_t numkeys = stoul(vStrInputParts[2]); + const uint32_t numkeys{TrimAndParse(vStrInputParts.at(2), "invalid multisig total number")}; // Validate there are the correct number of pubkeys if (vStrInputParts.size() < numkeys + 3) diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 36ebda774b4..cca5732aa15 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -579,6 +579,18 @@ "output_cmp": "txcreatedata_seq1.json", "description": "Adds a new input with sequence number to a transaction (output in json)" }, + { "exec": "./bitcoin-tx", + "args": ["-create", "outmultisig=1:-2:3:02a5:021:02df", "nversion=1"], + "return_code": 1, + "error_txt": "error: invalid multisig required number '-2'", + "description": "Try to parse a multisig number outside the allowed range" + }, + { "exec": "./bitcoin-tx", + "args": ["-create", "outmultisig=1:2:3a:02a5:021:02df", "nversion=1"], + "return_code": 1, + "error_txt": "error: invalid multisig total number '3a'", + "description": "Try to parse a multisig number outside the allowed range" + }, { "exec": "./bitcoin-tx", "args": ["-create", "outmultisig=1:2:3:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397:021ac43c7ff740014c3b33737ede99c967e4764553d1b2b83db77c83b8715fa72d:02df2089105c77f266fa11a9d33f05c735234075f2e8780824c6b709415f9fb485", "nversion=1"], "output_cmp": "txcreatemultisig1.hex",