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)

5 files changed, 101 insertions, 5 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/script/interpreter.cpp b/src/script/interpreter.cpp
index 9c47f7c6c..fd4a5674c 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -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;
diff --git a/src/script/script.h b/src/script/script.h
index d2a68a07b..2a338d6f5 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
@@ -621,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()
diff --git a/src/streams.h b/src/streams.h
index 0fc6135a6..ed14f3f41 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())
         {
@@ -404,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<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)
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index 48e3c8ed8..129ce04e0 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()