From 4c6ea562bd50d00594b9549d29f095fe5c38dfda Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 25 Apr 2016 12:31:45 +0200 Subject: [PATCH 1/6] Introduce constant for maximum CScript length --- src/script/interpreter.cpp | 2 +- src/script/script.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index ccc989577..19a6a8f4f 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -252,7 +252,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 79bd61630..8307aed2e 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -19,6 +19,9 @@ static const unsigned int MAX_SCRIPT_ELEMENT_SIZE = 520; // bytes +// 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 From 6fc5764eda115e0c6b348cab2cd5653641fb4a43 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 25 Apr 2016 12:32:01 +0200 Subject: [PATCH 2/6] Treat overly long scriptPubKeys as unspendable --- src/script/script.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/script.h b/src/script/script.h index 8307aed2e..e0e89185f 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -573,7 +573,7 @@ public: */ bool IsUnspendable() const { - return (size() > 0 && *begin() == OP_RETURN); + return (size() > 0 && *begin() == OP_RETURN) || (size() > MAX_SCRIPT_SIZE); } std::string ToString() const; From f588c5edd61f9b7d598f5885344c87300bcce690 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 24 Apr 2016 16:21:44 +0200 Subject: [PATCH 3/6] Fix OOM bug: UTXO entries with invalid script length --- src/compressor.h | 10 ++++++++-- src/streams.h | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/compressor.h b/src/compressor.h index 4a7209083..fa702f0df 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 787d8e297..fdba4622f 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) From e9d221e764be5731a785f063f3b9fce5a3bd4bfa Mon Sep 17 00:00:00 2001 From: Patrick Strateman Date: Sun, 24 Apr 2016 21:59:46 -0700 Subject: [PATCH 4/6] 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(-) diff --git a/src/streams.h b/src/streams.h index fdba4622f..9b678b3c0 100644 --- a/src/streams.h +++ b/src/streams.h @@ -241,7 +241,9 @@ public: CBaseDataStream& 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()) { From abd22bb674ce2ef584fc875fdb28255f25432a7b Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 25 Apr 2016 14:05:36 +0200 Subject: [PATCH 5/6] Add tests for CCoins deserialization --- src/test/coins_tests.cpp | 71 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 695a8b05d..029949dd4 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 "consensus/validation.h" #include "main.h" @@ -524,4 +526,73 @@ BOOST_AUTO_TEST_CASE(coins_coinbase_spends) } } +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() From 43f0769cf62e690a7601aefaf62e698417fd243c Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 20 Oct 2016 23:50:19 -0700 Subject: [PATCH 6/6] Fix build problem with coins_tests --- src/test/coins_tests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 029949dd4..8f626d9ab 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -11,6 +11,7 @@ #include "consensus/validation.h" #include "main.h" #include "undo.h" +#include "pubkey.h" #include #include