foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-02 09:46:52 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#23227: bitcoin-tx: Avoid treating integer overflow as OP_0

Browse source 
fa43e7c2d9 bitcoin-tx: Avoid treating overflow as OP_0 (MarcoFalke)
fa053c0019 style: Fix whitespace in Parse* functions (MarcoFalke)
fa03dec7e9 refactor: Use C++11 range based for loop in ParseScript (MarcoFalke)
fad55e79ca doc: Fixup ToIntegral docs (MarcoFalke)

Pull request description:

  Seems odd to treat integer overflow as `OP_0`, so fix that.

ACKs for top commit:
  theStack:
    re-ACK fa43e7c2d9
  shaavan:
    ACK fa43e7c2d9

Tree-SHA512: 1bbe2de62d853badc18d57d169c6e78ddcdff037e5a85357995dead11c8e67a4fe35087e08a181c60753f8ce91058b7fcc06f5b7901afedc78fbacea8bc3ef4f
This commit is contained in:
Samuel Dobson 2021-10-12 15:19:00 +13:00
commit fbbbc594ad
No known key found for this signature in database
GPG key ID: D300116E1C875A3D
5 changed files with 32 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

53
src/core_read.cpp
View file

@ -26,20 +26,20 @@ opcodetype ParseOpCode(const std::string& s)
{
static std::map<std::string, opcodetype> mapOpNames;
if (mapOpNames.empty())
{
for (unsigned int op = 0; op <= MAX_OPCODE; op++)
{
if (mapOpNames.empty()) {
for (unsigned int op = 0; op <= MAX_OPCODE; op++) {
// Allow OP_RESERVED to get into mapOpNames
if (op < OP_NOP && op != OP_RESERVED)
if (op < OP_NOP && op != OP_RESERVED) {
continue;
}
std::string strName = GetOpName(static_cast<opcodetype>(op));
if (strName == "OP_UNKNOWN")
if (strName == "OP_UNKNOWN") {
continue;
}
mapOpNames[strName] = static_cast<opcodetype>(op);
// Convenience: OP_ADD and just ADD are both recognized:
if (strName.compare(0, 3, "OP_") == 0) { // strName starts with "OP_"
if (strName.compare(0, 3, "OP_") == 0) { // strName starts with "OP_"
mapOpNames[strName.substr(3)] = static_cast<opcodetype>(op);
}
}
@ -59,44 +59,35 @@ CScript ParseScript(const std::string& s)
std::vector<std::string> words;
boost::algorithm::split(words, s, boost::algorithm::is_any_of(" \t\n"), boost::algorithm::token_compress_on);
for (std::vector<std::string>::const_iterator w = words.begin(); w != words.end(); ++w)
{
if (w->empty())
{
for (const std::string& w : words) {
if (w.empty()) {
// Empty string, ignore. (boost::split given '' will return one word)
}
else if (std::all_of(w->begin(), w->end(), ::IsDigit) ||
(w->front() == '-' && w->size() > 1 && std::all_of(w->begin()+1, w->end(), ::IsDigit)))
} else if (std::all_of(w.begin(), w.end(), ::IsDigit) ||
(w.front() == '-' && w.size() > 1 && std::all_of(w.begin() + 1, w.end(), ::IsDigit)))
{
// Number
int64_t n = LocaleIndependentAtoi<int64_t>(*w);
const auto num{ToIntegral<int64_t>(w)};
//limit the range of numbers ParseScript accepts in decimal
//since numbers outside -0xFFFFFFFF...0xFFFFFFFF are illegal in scripts
if (n > int64_t{0xffffffff} || n < -1 * int64_t{0xffffffff}) {
// limit the range of numbers ParseScript accepts in decimal
// since numbers outside -0xFFFFFFFF...0xFFFFFFFF are illegal in scripts
if (!num.has_value() || num > int64_t{0xffffffff} || num < -1 * int64_t{0xffffffff}) {
throw std::runtime_error("script parse error: decimal numeric value only allowed in the "
"range -0xFFFFFFFF...0xFFFFFFFF");
}
result << n;
}
else if (w->substr(0,2) == "0x" && w->size() > 2 && IsHex(std::string(w->begin()+2, w->end())))
{
result << num.value();
} else if (w.substr(0, 2) == "0x" && w.size() > 2 && IsHex(std::string(w.begin() + 2, w.end()))) {
// Raw hex data, inserted NOT pushed onto stack:
std::vector<unsigned char> raw = ParseHex(std::string(w->begin()+2, w->end()));
std::vector<unsigned char> raw = ParseHex(std::string(w.begin() + 2, w.end()));
result.insert(result.end(), raw.begin(), raw.end());
}
else if (w->size() >= 2 && w->front() == '\'' && w->back() == '\'')
{
} else if (w.size() >= 2 && w.front() == '\'' && w.back() == '\'') {
// Single-quoted string, pushed as data. NOTE: this is poor-man's
// parsing, spaces/tabs/newlines in single-quoted strings won't work.
std::vector<unsigned char> value(w->begin()+1, w->end()-1);
std::vector<unsigned char> value(w.begin() + 1, w.end() - 1);
result << value;
}
else
{
} else {
// opcode, e.g. OP_ADD or ADD:
result << ParseOpCode(*w);
result << ParseOpCode(w);
}
}

2
src/test/script_parse_tests.cpp
View file

@ -38,7 +38,6 @@ BOOST_AUTO_TEST_CASE(parse_script)
{"'17'", "023137"},
{"ELSE", "67"},
{"NOP10", "b9"},
{"11111111111111111111", "00"},
};
std::string all_in;
std::string all_out;
@ -49,6 +48,7 @@ BOOST_AUTO_TEST_CASE(parse_script)
}
BOOST_CHECK_EQUAL(HexStr(ParseScript(all_in)), all_out);
BOOST_CHECK_EXCEPTION(ParseScript("11111111111111111111"), std::runtime_error, HasReason("script parse error: decimal numeric value only allowed in the range -0xFFFFFFFF...0xFFFFFFFF"));
BOOST_CHECK_EXCEPTION(ParseScript("11111111111"), std::runtime_error, HasReason("script parse error: decimal numeric value only allowed in the range -0xFFFFFFFF...0xFFFFFFFF"));
BOOST_CHECK_EXCEPTION(ParseScript("OP_CHECKSIGADD"), std::runtime_error, HasReason("script parse error: unknown opcode"));
}

4
src/util/strencodings.h
View file

@ -72,7 +72,7 @@ void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut);
// LocaleIndependentAtoi is provided for backwards compatibility reasons.
//
// New code should use the ParseInt64/ParseUInt64/ParseInt32/ParseUInt32 functions
// New code should use ToIntegral or the ParseInt* functions
// which provide parse error feedback.
//
// The goal of LocaleIndependentAtoi is to replicate the exact defined behaviour
@ -125,7 +125,7 @@ constexpr inline bool IsSpace(char c) noexcept {
/**
* Convert string to integral type T. Leading whitespace, a leading +, or any
* trailing character fail the parsing. The required format expressed as regex
* is `-?[0-9]+`.
* is `-?[0-9]+`. The minus sign is only permitted for signed integer types.
*
* @returns std::nullopt if the entire string could not be parsed, or if the
* parsed value is not in the range representable by the type T.

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

@ -38,7 +38,7 @@ export LC_ALL=C
# https://stackoverflow.com/a/34878283 for more details.
# TODO: Reduce KNOWN_VIOLATIONS by replacing uses of locale dependent stoul/strtol with locale
# independent ToIntegral<T>(...).
# 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"

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

@ -294,6 +294,12 @@
"output_cmp": "txcreatescript4.json",
"description": "Create a new transaction with a single output script (OP_DROP) in a P2SH, wrapped in a P2SH (output as json)"
},
{ "exec": "./bitcoin-tx",
"args": ["-create", "outscript=0:999999999999999999999999999999"],
"return_code": 1,
"error_txt": "error: script parse error: decimal numeric value only allowed in the range -0xFFFFFFFF...0xFFFFFFFF",
"description": "Try to parse an output script with a decimal number above the allowed range"
},
{ "exec": "./bitcoin-tx",
"args": ["-create", "outscript=0:9999999999"],
"return_code": 1,