Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bitcoin-tx: Stricter check for valid integers #13603

Merged
merged 1 commit into from Jul 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 20 additions & 22 deletions src/bitcoin-tx.cpp
@@ -1,4 +1,4 @@
// Copyright (c) 2009-2017 The Bitcoin Core developers
// Copyright (c) 2009-2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand Down Expand Up @@ -193,27 +193,27 @@ static CAmount ExtractAndValidateValue(const std::string& strValue)

static void MutateTxVersion(CMutableTransaction& tx, const std::string& cmdVal)
{
int64_t newVersion = atoi64(cmdVal);
if (newVersion < 1 || newVersion > CTransaction::MAX_STANDARD_VERSION)
throw std::runtime_error("Invalid TX version requested");
int64_t newVersion;
if (!ParseInt64(cmdVal, &newVersion) || newVersion < 1 || newVersion > CTransaction::MAX_STANDARD_VERSION)
throw std::runtime_error("Invalid TX version requested: '" + cmdVal + "'");

tx.nVersion = (int) newVersion;
}

static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal)
{
int64_t newLocktime = atoi64(cmdVal);
if (newLocktime < 0LL || newLocktime > 0xffffffffLL)
throw std::runtime_error("Invalid TX locktime requested");
int64_t newLocktime;
if (!ParseInt64(cmdVal, &newLocktime) || newLocktime < 0LL || newLocktime > 0xffffffffLL)
throw std::runtime_error("Invalid TX locktime requested: '" + cmdVal + "'");

tx.nLockTime = (unsigned int) newLocktime;
}

static void MutateTxRBFOptIn(CMutableTransaction& tx, const std::string& strInIdx)
{
// parse requested index
int inIdx = atoi(strInIdx);
if (inIdx < 0 || inIdx >= (int)tx.vin.size()) {
int64_t inIdx;
if (!ParseInt64(strInIdx, &inIdx) || inIdx < 0 || inIdx >= static_cast<int64_t>(tx.vin.size())) {
throw std::runtime_error("Invalid TX input index '" + strInIdx + "'");
}

Expand Down Expand Up @@ -248,10 +248,10 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu
static const unsigned int maxVout = MAX_BLOCK_WEIGHT / (WITNESS_SCALE_FACTOR * minTxOutSz);

// extract and validate vout
std::string strVout = vStrInputParts[1];
int vout = atoi(strVout);
if ((vout < 0) || (vout > (int)maxVout))
throw std::runtime_error("invalid TX input vout");
const std::string& strVout = vStrInputParts[1];
int64_t vout;
if (!ParseInt64(strVout, &vout) || vout < 0 || vout > static_cast<int64_t>(maxVout))
throw std::runtime_error("invalid TX input vout '" + strVout + "'");

// extract the optional sequence number
uint32_t nSequenceIn=std::numeric_limits<unsigned int>::max();
Expand Down Expand Up @@ -481,10 +481,9 @@ static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& str
static void MutateTxDelInput(CMutableTransaction& tx, const std::string& strInIdx)
{
// parse requested deletion index
int inIdx = atoi(strInIdx);
if (inIdx < 0 || inIdx >= (int)tx.vin.size()) {
std::string strErr = "Invalid TX input index '" + strInIdx + "'";
throw std::runtime_error(strErr.c_str());
int64_t inIdx;
if (!ParseInt64(strInIdx, &inIdx) || inIdx < 0 || inIdx >= static_cast<int64_t>(tx.vin.size())) {
throw std::runtime_error("Invalid TX input index '" + strInIdx + "'");
}

// delete input from transaction
Expand All @@ -494,10 +493,9 @@ static void MutateTxDelInput(CMutableTransaction& tx, const std::string& strInId
static void MutateTxDelOutput(CMutableTransaction& tx, const std::string& strOutIdx)
{
// parse requested deletion index
int outIdx = atoi(strOutIdx);
if (outIdx < 0 || outIdx >= (int)tx.vout.size()) {
std::string strErr = "Invalid TX output index '" + strOutIdx + "'";
throw std::runtime_error(strErr.c_str());
int64_t outIdx;
if (!ParseInt64(strOutIdx, &outIdx) || outIdx < 0 || outIdx >= static_cast<int64_t>(tx.vout.size())) {
throw std::runtime_error("Invalid TX output index '" + strOutIdx + "'");
}

// delete output from transaction
Expand Down Expand Up @@ -593,7 +591,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)

uint256 txid = ParseHashStr(prevOut["txid"].get_str(), "txid");

int nOut = atoi(prevOut["vout"].getValStr());
const int nOut = prevOut["vout"].get_int();
if (nOut < 0)
throw std::runtime_error("vout must be positive");

Expand Down
55 changes: 55 additions & 0 deletions test/util/data/bitcoin-util-test.json
Expand Up @@ -26,6 +26,12 @@
"output_cmp": "blanktxv2.json",
"description": "Creates a blank transaction when nothing is piped into bitcoin-tx (output in json)"
},
{ "exec": "./bitcoin-tx",
"args": ["-create", "nversion=1foo"],
"return_code": 1,
"error_txt": "error: Invalid TX version requested",
"description": "Tests the check for invalid nversion value"
},
{ "exec": "./bitcoin-tx",
"args": ["-", "delin=1"],
"input": "tx394b54bb.hex",
Expand All @@ -45,6 +51,13 @@
"error_txt": "error: Invalid TX input index '31'",
"description": "Attempts to delete an input with a bad index from a transaction. Expected to fail."
},
{ "exec": "./bitcoin-tx",
"args": ["-", "delin=1foo"],
"input": "tx394b54bb.hex",
"return_code": 1,
"error_txt": "error: Invalid TX input index",
"description": "Tests the check for an invalid input index with delin"
},
{ "exec": "./bitcoin-tx",
"args": ["-", "delout=1"],
"input": "tx394b54bb.hex",
Expand All @@ -64,6 +77,13 @@
"error_txt": "error: Invalid TX output index '2'",
"description": "Attempts to delete an output with a bad index from a transaction. Expected to fail."
},
{ "exec": "./bitcoin-tx",
"args": ["-", "delout=1foo"],
"input": "tx394b54bb.hex",
"return_code": 1,
"error_txt": "error: Invalid TX output index",
"description": "Tests the check for an invalid output index with delout"
},
{ "exec": "./bitcoin-tx",
"args": ["-", "locktime=317000"],
"input": "tx394b54bb.hex",
Expand All @@ -76,6 +96,29 @@
"output_cmp": "tt-locktime317000-out.json",
"description": "Adds an nlocktime to a transaction (output in json)"
},
{ "exec": "./bitcoin-tx",
"args": ["-create", "locktime=317000foo"],
"return_code": 1,
"error_txt": "error: Invalid TX locktime requested",
"description": "Tests the check for invalid locktime value"
},
{ "exec": "./bitcoin-tx",
"args":
["-create",
"in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0",
"replaceable=0foo"],
"return_code": 1,
"error_txt": "error: Invalid TX input index",
"description": "Tests the check for an invalid input index with replaceable"
},
{ "exec": "./bitcoin-tx",
"args":
["-create",
"in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0x"],
"return_code": 1,
"error_txt": "error: invalid TX input vout",
"description": "Tests the check for an invalid vout value when adding an input"
},
{ "exec": "./bitcoin-tx",
"args":
["-create",
Expand Down Expand Up @@ -225,6 +268,18 @@
"output_cmp": "txcreatesignv2.hex",
"description": "Creates a new transaction with a single input and a single output, and then signs the transaction"
},
{ "exec": "./bitcoin-tx",
"args":
["-create",
"in=4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485:0",
"set=privatekeys:[\"5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAnchuDf\"]",
"set=prevtxs:[{\"txid\":\"4d49a71ec9da436f71ec4ee231d04f292a29cd316f598bb7068feccabdc59485\",\"vout\":\"0foo\",\"scriptPubKey\":\"76a91491b24bf9f5288532960ac687abb035127b1d28a588ac\"}]",
"sign=ALL",
"outaddr=0.001:193P6LtvS4nCnkDvM9uXn1gsSRqh4aDAz7"],
"return_code": 1,
"error_txt": "error: prevtxs internal object typecheck fail",
"description": "Tests the check for invalid vout index in prevtxs for sign"
},
{ "exec": "./bitcoin-tx",
"args":
["-create", "outpubkey=0:02a5613bd857b7048924264d1e70e08fb2a7e6527d32b7ab1bb993ac59964ff397", "nversion=1"],
Expand Down