Skip to content

Commit

Permalink
Merge bitcoin#7933: Fix OOM when deserializing UTXO entries with inva…
Browse files Browse the repository at this point in the history 
…lid length

1e44169 Add tests for CCoins deserialization (Pieter Wuille)
5d0434d Fix OOM bug: UTXO entries with invalid script length (Pieter Wuille)
4bf631e CDataStream::ignore Throw exception instead of assert on negative nSize. (Patrick Strateman)
4f87af6 Treat overly long scriptPubKeys as unspendable (Pieter Wuille)
f8e6fb1 Introduce constant for maximum CScript length (Pieter Wuille)
  • Loading branch information
@laanwj @codablock
laanwj authored and codablock committed Oct 19, 2017
1 parent 753cb15 commit 952383e
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 5 deletions.
10 changes: 8 additions & 2 deletions src/compressor.h
View file
Expand Up @@ -86,8 +86,14 @@ class CScriptCompressor
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));
}
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/script/interpreter.cpp
View file
Expand Up @@ -247,7 +247,7 @@ bool EvalScript(vector<vector<unsigned char> >& stack, const CScript& script, un
vector<bool> vfExec;
vector<valtype> 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;
Expand Down
5 changes: 4 additions & 1 deletion src/script/script.h
View file
Expand Up @@ -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
Expand Down Expand Up @@ -633,7 +636,7 @@ class CScript : public CScriptBase
*/
bool IsUnspendable() const
{
return (size() > 0 && *begin() == OP_RETURN);
return (size() > 0 && *begin() == OP_RETURN) || (size() > MAX_SCRIPT_SIZE);
}

void clear()
Expand Down
18 changes: 17 additions & 1 deletion src/streams.h
View file
Expand Up @@ -247,7 +247,9 @@ class CDataStream
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())
{
Expand Down Expand Up @@ -411,6 +413,20 @@ class CAutoFile
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<size_t>(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)
Expand Down
71 changes: 71 additions & 0 deletions src/test/coins_tests.cpp
View file
Expand Up @@ -4,7 +4,9 @@

#include "coins.h"
#include "random.h"
#include "script/standard.h"
#include "uint256.h"
#include "utilstrencodings.h"
#include "test/test_dash.h"
#include "validation.h"
#include "consensus/validation.h"
Expand Down Expand Up @@ -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()

0 comments on commit 952383e

Please sign in to comment.