foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-05 14:06:27 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#23253: bitcoin-tx: Reject non-integral and out of range int strings

Browse source 
fa6f29de51 bitcoin-tx: Reject non-integral and out of range multisig numbers (MarcoFalke)
fafab8ea5e bitcoin-tx: Reject non-integral and out of range sequence ids (MarcoFalke)
fa53d3d826 test: Check that bitcoin-tx accepts whitespace around sequence id and multisig numbers (MarcoFalke)

Pull request description:

  Seems odd to silently accept arbitrary strings that don't even represent integral values.

  Fix that.

ACKs for top commit:
  practicalswift:
    cr ACK fa6f29de51
  laanwj:
    Code review ACK fa6f29de51
  Empact:
    Code review ACK fa6f29de51
  promag:
    Code review ACK fa6f29de51.

Tree-SHA512: e31f7f21fe55ac069e755557bdbcae8d5d29e20ff82e441ebdfc65153e3a31a4edd46ad3e6dea5190ecbd1b8ea5a8f94daa5d59a3b7558e46e794e30db0e6c79
This commit is contained in:
W. J. van der Laan 2021-10-13 13:48:36 +02:00
commit 28d5074343
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
3 changed files with 61 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
src/bitcoin-tx.cpp
View file

@ -235,6 +235,16 @@ static void MutateTxRBFOptIn(CMutableTransaction& tx, const std::string& strInId
}
}
template <typename T>
static T TrimAndParse(const std::string& int_str, const std::string& err)
{
const auto parsed{ToIntegral<T>(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<std::string> 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<uint32_t>(vStrInputParts.at(2), "invalid TX sequence id");
}
// append to transaction input list
CTxIn txin(txid, vout, CScript(), nSequenceIn);
@ -352,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<uint32_t>(vStrInputParts.at(1), "invalid multisig required number")};
// Extract NUMKEYS
uint32_t numkeys = stoul(vStrInputParts[2]);
const uint32_t numkeys{TrimAndParse<uint32_t>(vStrInputParts.at(2), "invalid multisig total number")};
// Validate there are the correct number of pubkeys
if (vStrInputParts.size() < numkeys + 3)

1
test/lint/lint-locale-dependence.sh
View file

@ -41,7 +41,6 @@ export LC_ALL=C
# independent ToIntegral<T>(...) 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/test/dbwrapper_tests.cpp:.*snprintf"
"src/test/fuzz/locale.cpp"

48
test/util/data/bitcoin-util-test.json
View file

@ -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",
@ -523,6 +547,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",
@ -547,15 +579,27 @@
"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",
"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"],