From 57889e688dd0987a1e087cd48d216a413127601e Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Fri, 6 Jul 2018 15:54:03 +0200 Subject: bitcoin-tx: Stricter check for valid integers Just calling atoi to convert strings to integers does not check for valid integers very thoroughly; in particular, it just ignores everything starting from the first non-numeral character. Even a string like "foo" is fine and silently returns 0. This meant that bitcoin-tx would not fail if such a string was passed in various places where an integer is expected (like the locktime or an input/output index); this means that it would, for instance, silently accept a typo and interpret it in an unexpected way. In this change, we use ParseInt64 for parsing strings to integers, which actually verifies that the full string is valid as number. New tests in the bitcoin-util-test cover the new error paths. --- src/bitcoin-tx.cpp | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) (limited to 'src/bitcoin-tx.cpp') diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index ab0efde05c..181e2bb1bc 100644 --- a/src/bitcoin-tx.cpp +++ b/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. @@ -193,18 +193,18 @@ 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; } @@ -212,8 +212,8 @@ static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal) 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(tx.vin.size())) { throw std::runtime_error("Invalid TX input index '" + strInIdx + "'"); } @@ -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(maxVout)) + throw std::runtime_error("invalid TX input vout '" + strVout + "'"); // extract the optional sequence number uint32_t nSequenceIn=std::numeric_limits::max(); @@ -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(tx.vin.size())) { + throw std::runtime_error("Invalid TX input index '" + strInIdx + "'"); } // delete input from transaction @@ -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(tx.vout.size())) { + throw std::runtime_error("Invalid TX output index '" + strOutIdx + "'"); } // delete output from transaction @@ -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"); -- cgit v1.2.3