From f8e6fb1800fbac87e76cdddc074d8f4af585f050 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 25 Apr 2016 12:31:45 +0200 Subject: Introduce constant for maximum CScript length --- src/script/interpreter.cpp | 2 +- src/script/script.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 9c47f7c6c9..fd4a5674cf 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -247,7 +247,7 @@ bool EvalScript(vector >& stack, const CScript& script, un vector vfExec; vector altstack; set_error(serror, SCRIPT_ERR_UNKNOWN_ERROR); - if (script.size() > 10000) + if (script.size() > MAX_SCRIPT_SIZE) return set_error(serror, SCRIPT_ERR_SCRIPT_SIZE); int nOpCount = 0; bool fRequireMinimal = (flags & SCRIPT_VERIFY_MINIMALDATA) != 0; diff --git a/src/script/script.h b/src/script/script.h index d2a68a07ba..68cde03e34 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -27,6 +27,9 @@ static const int MAX_OPS_PER_SCRIPT = 201; // Maximum number of public keys per multisig static const int MAX_PUBKEYS_PER_MULTISIG = 20; +// Maximum script length in bytes +static const int MAX_SCRIPT_SIZE = 10000; + // Threshold for nLockTime: below this value it is interpreted as block number, // otherwise as UNIX timestamp. static const unsigned int LOCKTIME_THRESHOLD = 500000000; // Tue Nov 5 00:53:20 1985 UTC -- cgit v1.2.3 From 4f87af6fc7580912726f9bf833c21e6e1b478e1d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 25 Apr 2016 12:32:01 +0200 Subject: Treat overly long scriptPubKeys as unspendable --- src/script/script.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/script/script.h b/src/script/script.h index 68cde03e34..2a338d6f5c 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -624,7 +624,7 @@ public: */ bool IsUnspendable() const { - return (size() > 0 && *begin() == OP_RETURN); + return (size() > 0 && *begin() == OP_RETURN) || (size() > MAX_SCRIPT_SIZE); } void clear() -- cgit v1.2.3 From 4bf631e5e48cd4c14c825cdaf7a1bee81e15493d Mon Sep 17 00:00:00 2001 From: Patrick Strateman Date: Sun, 24 Apr 2016 21:59:46 -0700 Subject: CDataStream::ignore Throw exception instead of assert on negative nSize. Previously disk corruption would cause an assert instead of an exception. --- src/streams.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/streams.h b/src/streams.h index 0fc6135a6a..a50fe4e859 100644 --- a/src/streams.h +++ b/src/streams.h @@ -240,7 +240,9 @@ public: CDataStream& ignore(int nSize) { // Ignore from the beginning of the buffer - assert(nSize >= 0); + if (nSize < 0) { + throw std::ios_base::failure("CDataStream::ignore(): nSize negative"); + } unsigned int nReadPosNext = nReadPos + nSize; if (nReadPosNext >= vch.size()) { -- cgit v1.2.3 From 5d0434d13d0145a110c0c93e59edfd7d062f8531 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 24 Apr 2016 16:21:44 +0200 Subject: Fix OOM bug: UTXO entries with invalid script length --- src/compressor.h | 10 ++++++++-- src/streams.h | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/compressor.h b/src/compressor.h index 4a72090830..fa702f0dfa 100644 --- a/src/compressor.h +++ b/src/compressor.h @@ -86,8 +86,14 @@ public: return; } nSize -= nSpecialScripts; - script.resize(nSize); - s >> REF(CFlatData(script)); + if (nSize > MAX_SCRIPT_SIZE) { + // Overly long script, replace with a short invalid one + script << OP_RETURN; + s.ignore(nSize); + } else { + script.resize(nSize); + s >> REF(CFlatData(script)); + } } }; diff --git a/src/streams.h b/src/streams.h index a50fe4e859..ed14f3f412 100644 --- a/src/streams.h +++ b/src/streams.h @@ -406,6 +406,20 @@ public: return (*this); } + CAutoFile& ignore(size_t nSize) + { + if (!file) + throw std::ios_base::failure("CAutoFile::ignore: file handle is NULL"); + unsigned char data[4096]; + while (nSize > 0) { + size_t nNow = std::min(nSize, sizeof(data)); + if (fread(data, 1, nNow, file) != nNow) + throw std::ios_base::failure(feof(file) ? "CAutoFile::ignore: end of file" : "CAutoFile::read: fread failed"); + nSize -= nNow; + } + return (*this); + } + CAutoFile& write(const char* pch, size_t nSize) { if (!file) -- cgit v1.2.3 From 1e44169f0e0c334a86b14a22ebc8fec45cec7354 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 25 Apr 2016 14:05:36 +0200 Subject: Add tests for CCoins deserialization --- src/test/coins_tests.cpp | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) (limited to 'src') diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 48e3c8ed8e..129ce04e0b 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -4,7 +4,9 @@ #include "coins.h" #include "random.h" +#include "script/standard.h" #include "uint256.h" +#include "utilstrencodings.h" #include "test/test_bitcoin.h" #include "main.h" #include "consensus/validation.h" @@ -345,4 +347,73 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) BOOST_CHECK(spent_a_duplicate_coinbase); } +BOOST_AUTO_TEST_CASE(ccoins_serialization) +{ + // Good example + CDataStream ss1(ParseHex("0104835800816115944e077fe7c803cfa57f29b36bf87c1d358bb85e"), SER_DISK, CLIENT_VERSION); + CCoins cc1; + ss1 >> cc1; + BOOST_CHECK_EQUAL(cc1.nVersion, 1); + BOOST_CHECK_EQUAL(cc1.fCoinBase, false); + BOOST_CHECK_EQUAL(cc1.nHeight, 203998); + BOOST_CHECK_EQUAL(cc1.vout.size(), 2); + BOOST_CHECK_EQUAL(cc1.IsAvailable(0), false); + BOOST_CHECK_EQUAL(cc1.IsAvailable(1), true); + BOOST_CHECK_EQUAL(cc1.vout[1].nValue, 60000000000ULL); + BOOST_CHECK_EQUAL(HexStr(cc1.vout[1].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("816115944e077fe7c803cfa57f29b36bf87c1d35")))))); + + // Good example + CDataStream ss2(ParseHex("0109044086ef97d5790061b01caab50f1b8e9c50a5057eb43c2d9563a4eebbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa486af3b"), SER_DISK, CLIENT_VERSION); + CCoins cc2; + ss2 >> cc2; + BOOST_CHECK_EQUAL(cc2.nVersion, 1); + BOOST_CHECK_EQUAL(cc2.fCoinBase, true); + BOOST_CHECK_EQUAL(cc2.nHeight, 120891); + BOOST_CHECK_EQUAL(cc2.vout.size(), 17); + for (int i = 0; i < 17; i++) { + BOOST_CHECK_EQUAL(cc2.IsAvailable(i), i == 4 || i == 16); + } + BOOST_CHECK_EQUAL(cc2.vout[4].nValue, 234925952); + BOOST_CHECK_EQUAL(HexStr(cc2.vout[4].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("61b01caab50f1b8e9c50a5057eb43c2d9563a4ee")))))); + BOOST_CHECK_EQUAL(cc2.vout[16].nValue, 110397); + BOOST_CHECK_EQUAL(HexStr(cc2.vout[16].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("8c988f1a4a4de2161e0f50aac7f17e7f9555caa4")))))); + + // Smallest possible example + CDataStream ssx(SER_DISK, CLIENT_VERSION); + BOOST_CHECK_EQUAL(HexStr(ssx.begin(), ssx.end()), ""); + + CDataStream ss3(ParseHex("0002000600"), SER_DISK, CLIENT_VERSION); + CCoins cc3; + ss3 >> cc3; + BOOST_CHECK_EQUAL(cc3.nVersion, 0); + BOOST_CHECK_EQUAL(cc3.fCoinBase, false); + BOOST_CHECK_EQUAL(cc3.nHeight, 0); + BOOST_CHECK_EQUAL(cc3.vout.size(), 1); + BOOST_CHECK_EQUAL(cc3.IsAvailable(0), true); + BOOST_CHECK_EQUAL(cc3.vout[0].nValue, 0); + BOOST_CHECK_EQUAL(cc3.vout[0].scriptPubKey.size(), 0); + + // scriptPubKey that ends beyond the end of the stream + CDataStream ss4(ParseHex("0002000800"), SER_DISK, CLIENT_VERSION); + try { + CCoins cc4; + ss4 >> cc4; + BOOST_CHECK_MESSAGE(false, "We should have thrown"); + } catch (const std::ios_base::failure& e) { + } + + // Very large scriptPubKey (3*10^9 bytes) past the end of the stream + CDataStream tmp(SER_DISK, CLIENT_VERSION); + uint64_t x = 3000000000ULL; + tmp << VARINT(x); + BOOST_CHECK_EQUAL(HexStr(tmp.begin(), tmp.end()), "8a95c0bb00"); + CDataStream ss5(ParseHex("0002008a95c0bb0000"), SER_DISK, CLIENT_VERSION); + try { + CCoins cc5; + ss5 >> cc5; + BOOST_CHECK_MESSAGE(false, "We should have thrown"); + } catch (const std::ios_base::failure& e) { + } +} + BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3