Skip to content

Commit e26b620

Browse files
committed
Merge #7933: Fix OOM when deserializing UTXO entries with invalid 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)
2 parents 0ea3941 + 1e44169 commit e26b620

File tree

5 files changed

+101
-5
lines changed

5 files changed

+101
-5
lines changed

src/compressor.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,14 @@ class CScriptCompressor
8686
return;
8787
}
8888
nSize -= nSpecialScripts;
89-
script.resize(nSize);
90-
s >> REF(CFlatData(script));
89+
if (nSize > MAX_SCRIPT_SIZE) {
90+
// Overly long script, replace with a short invalid one
91+
script << OP_RETURN;
92+
s.ignore(nSize);
93+
} else {
94+
script.resize(nSize);
95+
s >> REF(CFlatData(script));
96+
}
9197
}
9298
};
9399

src/script/interpreter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ bool EvalScript(vector<vector<unsigned char> >& stack, const CScript& script, un
247247
vector<bool> vfExec;
248248
vector<valtype> altstack;
249249
set_error(serror, SCRIPT_ERR_UNKNOWN_ERROR);
250-
if (script.size() > 10000)
250+
if (script.size() > MAX_SCRIPT_SIZE)
251251
return set_error(serror, SCRIPT_ERR_SCRIPT_SIZE);
252252
int nOpCount = 0;
253253
bool fRequireMinimal = (flags & SCRIPT_VERIFY_MINIMALDATA) != 0;

src/script/script.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ static const int MAX_OPS_PER_SCRIPT = 201;
2727
// Maximum number of public keys per multisig
2828
static const int MAX_PUBKEYS_PER_MULTISIG = 20;
2929

30+
// Maximum script length in bytes
31+
static const int MAX_SCRIPT_SIZE = 10000;
32+
3033
// Threshold for nLockTime: below this value it is interpreted as block number,
3134
// otherwise as UNIX timestamp.
3235
static const unsigned int LOCKTIME_THRESHOLD = 500000000; // Tue Nov 5 00:53:20 1985 UTC
@@ -621,7 +624,7 @@ class CScript : public CScriptBase
621624
*/
622625
bool IsUnspendable() const
623626
{
624-
return (size() > 0 && *begin() == OP_RETURN);
627+
return (size() > 0 && *begin() == OP_RETURN) || (size() > MAX_SCRIPT_SIZE);
625628
}
626629

627630
void clear()

src/streams.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,9 @@ class CDataStream
240240
CDataStream& ignore(int nSize)
241241
{
242242
// Ignore from the beginning of the buffer
243-
assert(nSize >= 0);
243+
if (nSize < 0) {
244+
throw std::ios_base::failure("CDataStream::ignore(): nSize negative");
245+
}
244246
unsigned int nReadPosNext = nReadPos + nSize;
245247
if (nReadPosNext >= vch.size())
246248
{
@@ -404,6 +406,20 @@ class CAutoFile
404406
return (*this);
405407
}
406408

409+
CAutoFile& ignore(size_t nSize)
410+
{
411+
if (!file)
412+
throw std::ios_base::failure("CAutoFile::ignore: file handle is NULL");
413+
unsigned char data[4096];
414+
while (nSize > 0) {
415+
size_t nNow = std::min<size_t>(nSize, sizeof(data));
416+
if (fread(data, 1, nNow, file) != nNow)
417+
throw std::ios_base::failure(feof(file) ? "CAutoFile::ignore: end of file" : "CAutoFile::read: fread failed");
418+
nSize -= nNow;
419+
}
420+
return (*this);
421+
}
422+
407423
CAutoFile& write(const char* pch, size_t nSize)
408424
{
409425
if (!file)

src/test/coins_tests.cpp

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44

55
#include "coins.h"
66
#include "random.h"
7+
#include "script/standard.h"
78
#include "uint256.h"
9+
#include "utilstrencodings.h"
810
#include "test/test_bitcoin.h"
911
#include "main.h"
1012
#include "consensus/validation.h"
@@ -345,4 +347,73 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test)
345347
BOOST_CHECK(spent_a_duplicate_coinbase);
346348
}
347349

350+
BOOST_AUTO_TEST_CASE(ccoins_serialization)
351+
{
352+
// Good example
353+
CDataStream ss1(ParseHex("0104835800816115944e077fe7c803cfa57f29b36bf87c1d358bb85e"), SER_DISK, CLIENT_VERSION);
354+
CCoins cc1;
355+
ss1 >> cc1;
356+
BOOST_CHECK_EQUAL(cc1.nVersion, 1);
357+
BOOST_CHECK_EQUAL(cc1.fCoinBase, false);
358+
BOOST_CHECK_EQUAL(cc1.nHeight, 203998);
359+
BOOST_CHECK_EQUAL(cc1.vout.size(), 2);
360+
BOOST_CHECK_EQUAL(cc1.IsAvailable(0), false);
361+
BOOST_CHECK_EQUAL(cc1.IsAvailable(1), true);
362+
BOOST_CHECK_EQUAL(cc1.vout[1].nValue, 60000000000ULL);
363+
BOOST_CHECK_EQUAL(HexStr(cc1.vout[1].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("816115944e077fe7c803cfa57f29b36bf87c1d35"))))));
364+
365+
// Good example
366+
CDataStream ss2(ParseHex("0109044086ef97d5790061b01caab50f1b8e9c50a5057eb43c2d9563a4eebbd123008c988f1a4a4de2161e0f50aac7f17e7f9555caa486af3b"), SER_DISK, CLIENT_VERSION);
367+
CCoins cc2;
368+
ss2 >> cc2;
369+
BOOST_CHECK_EQUAL(cc2.nVersion, 1);
370+
BOOST_CHECK_EQUAL(cc2.fCoinBase, true);
371+
BOOST_CHECK_EQUAL(cc2.nHeight, 120891);
372+
BOOST_CHECK_EQUAL(cc2.vout.size(), 17);
373+
for (int i = 0; i < 17; i++) {
374+
BOOST_CHECK_EQUAL(cc2.IsAvailable(i), i == 4 || i == 16);
375+
}
376+
BOOST_CHECK_EQUAL(cc2.vout[4].nValue, 234925952);
377+
BOOST_CHECK_EQUAL(HexStr(cc2.vout[4].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("61b01caab50f1b8e9c50a5057eb43c2d9563a4ee"))))));
378+
BOOST_CHECK_EQUAL(cc2.vout[16].nValue, 110397);
379+
BOOST_CHECK_EQUAL(HexStr(cc2.vout[16].scriptPubKey), HexStr(GetScriptForDestination(CKeyID(uint160(ParseHex("8c988f1a4a4de2161e0f50aac7f17e7f9555caa4"))))));
380+
381+
// Smallest possible example
382+
CDataStream ssx(SER_DISK, CLIENT_VERSION);
383+
BOOST_CHECK_EQUAL(HexStr(ssx.begin(), ssx.end()), "");
384+
385+
CDataStream ss3(ParseHex("0002000600"), SER_DISK, CLIENT_VERSION);
386+
CCoins cc3;
387+
ss3 >> cc3;
388+
BOOST_CHECK_EQUAL(cc3.nVersion, 0);
389+
BOOST_CHECK_EQUAL(cc3.fCoinBase, false);
390+
BOOST_CHECK_EQUAL(cc3.nHeight, 0);
391+
BOOST_CHECK_EQUAL(cc3.vout.size(), 1);
392+
BOOST_CHECK_EQUAL(cc3.IsAvailable(0), true);
393+
BOOST_CHECK_EQUAL(cc3.vout[0].nValue, 0);
394+
BOOST_CHECK_EQUAL(cc3.vout[0].scriptPubKey.size(), 0);
395+
396+
// scriptPubKey that ends beyond the end of the stream
397+
CDataStream ss4(ParseHex("0002000800"), SER_DISK, CLIENT_VERSION);
398+
try {
399+
CCoins cc4;
400+
ss4 >> cc4;
401+
BOOST_CHECK_MESSAGE(false, "We should have thrown");
402+
} catch (const std::ios_base::failure& e) {
403+
}
404+
405+
// Very large scriptPubKey (3*10^9 bytes) past the end of the stream
406+
CDataStream tmp(SER_DISK, CLIENT_VERSION);
407+
uint64_t x = 3000000000ULL;
408+
tmp << VARINT(x);
409+
BOOST_CHECK_EQUAL(HexStr(tmp.begin(), tmp.end()), "8a95c0bb00");
410+
CDataStream ss5(ParseHex("0002008a95c0bb0000"), SER_DISK, CLIENT_VERSION);
411+
try {
412+
CCoins cc5;
413+
ss5 >> cc5;
414+
BOOST_CHECK_MESSAGE(false, "We should have thrown");
415+
} catch (const std::ios_base::failure& e) {
416+
}
417+
}
418+
348419
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)