fix: bound CoinJoin entry intake

Author: thepastaclaw

src/coinjoin/coinjoin.h

@@ -154,6 +154,7 @@ class CCoinJoinEntry
     CTransactionRef txCollateral;
     // memory only
     CService addr;
+    bool fHasOversizedTxOut{false};
 
     CCoinJoinEntry() :
         txCollateral(MakeTransactionRef(CMutableTransaction{}))
@@ -167,9 +168,41 @@ class CCoinJoinEntry
     {
     }
 
-    SERIALIZE_METHODS(CCoinJoinEntry, obj)
+    template <typename Stream>
+    void Serialize(Stream& s) const
     {
-        READWRITE(obj.vecTxDSIn, obj.txCollateral, obj.vecTxOut);
+        s << vecTxDSIn << txCollateral << vecTxOut;
+    }
+
+    template <typename Stream>
+    void Unserialize(Stream& s)
+    {
+        vecTxDSIn.clear();
+        vecTxOut.clear();
+        fHasOversizedTxOut = false;
+
+        const size_t txdsin_size = ReadCompactSize(s);
+        if (txdsin_size > COINJOIN_ENTRY_MAX_SIZE) {
+            throw std::ios_base::failure("CCoinJoinEntry::vecTxDSIn size too large");
+        }
+        vecTxDSIn.reserve(txdsin_size);
+        while (vecTxDSIn.size() < txdsin_size) {
+            vecTxDSIn.emplace_back();
+            s >> vecTxDSIn.back();
+        }
+
+        s >> txCollateral;
+
+        const size_t txout_size = ReadCompactSize(s);
+        if (txout_size > COINJOIN_ENTRY_MAX_SIZE) {
+            fHasOversizedTxOut = true;
+            return;
+        }
+        vecTxOut.reserve(txout_size);
+        while (vecTxOut.size() < txout_size) {
+            vecTxOut.emplace_back();
+            s >> vecTxOut.back();
+        }
     }
 
     bool AddScriptSig(const CTxIn& txin);

src/coinjoin/server.cpp

@@ -592,19 +592,28 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag
         return false;
     }
 
-    if (!CoinJoin::IsCollateralValid(m_chainman, m_isman, mempool, *entry.txCollateral)) {
-        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: collateral not valid!\n", __func__);
-        nMessageIDRet = ERR_INVALID_COLLATERAL;
+    if (entry.vecTxDSIn.size() > COINJOIN_ENTRY_MAX_SIZE) {
+        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: too many inputs! %d/%d\n", __func__, entry.vecTxDSIn.size(), COINJOIN_ENTRY_MAX_SIZE);
+        nMessageIDRet = ERR_MAXIMUM;
+        ConsumeCollateral(entry.txCollateral);
         return false;
     }
 
-    if (entry.vecTxDSIn.size() > COINJOIN_ENTRY_MAX_SIZE) {
-        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: too many inputs! %d/%d\n", __func__, entry.vecTxDSIn.size(), COINJOIN_ENTRY_MAX_SIZE);
+    if (entry.fHasOversizedTxOut || entry.vecTxOut.size() > COINJOIN_ENTRY_MAX_SIZE) {
+        const size_t txout_size{entry.fHasOversizedTxOut ? COINJOIN_ENTRY_MAX_SIZE + 1 : entry.vecTxOut.size()};
+        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: too many outputs! %d/%d\n", __func__,
+                 static_cast<int>(txout_size), static_cast<int>(COINJOIN_ENTRY_MAX_SIZE));
         nMessageIDRet = ERR_MAXIMUM;
         ConsumeCollateral(entry.txCollateral);
         return false;
     }
 
+    if (!CoinJoin::IsCollateralValid(m_chainman, m_isman, mempool, *entry.txCollateral)) {
+        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: collateral not valid!\n", __func__);
+        nMessageIDRet = ERR_INVALID_COLLATERAL;
+        return false;
+    }
+
     std::vector<CTxIn> vin;
     for (const auto& txin : entry.vecTxDSIn) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- txin=%s\n", __func__, txin.ToString());

src/test/coinjoin_inouts_tests.cpp

@@ -14,6 +14,7 @@
 #include <coinjoin/common.h>
 #include <evo/chainhelper.h>
 #include <script/script.h>
+#include <streams.h>
 #include <uint256.h>
 #include <util/check.h>
 #include <util/time.h>
@@ -155,6 +156,44 @@ BOOST_AUTO_TEST_CASE(entry_addscriptsig_matches_and_rejects)
     }
 }
 
+BOOST_AUTO_TEST_CASE(entry_rejects_oversized_inputs_before_materializing)
+{
+    CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
+    WriteCompactSize(stream, COINJOIN_ENTRY_MAX_SIZE + 1);
+
+    CCoinJoinEntry entry;
+    BOOST_CHECK_THROW(stream >> entry, std::ios_base::failure);
+    BOOST_CHECK(entry.vecTxDSIn.empty());
+}
+
+BOOST_AUTO_TEST_CASE(entry_flags_oversized_outputs_before_materializing)
+{
+    CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
+    WriteCompactSize(stream, 0);
+    stream << MakeTransactionRef(CMutableTransaction{});
+    WriteCompactSize(stream, COINJOIN_ENTRY_MAX_SIZE + 1);
+
+    CCoinJoinEntry entry;
+    BOOST_CHECK_NO_THROW(stream >> entry);
+    BOOST_CHECK(entry.fHasOversizedTxOut);
+    BOOST_CHECK(entry.vecTxOut.empty());
+}
+
+BOOST_AUTO_TEST_CASE(entry_accepts_max_sized_vectors)
+{
+    CCoinJoinEntry entry;
+    entry.vecTxDSIn.resize(COINJOIN_ENTRY_MAX_SIZE); // exactly the cap must be accepted
+    entry.vecTxOut.resize(COINJOIN_ENTRY_MAX_SIZE);
+
+    CDataStream stream{SER_NETWORK, PROTOCOL_VERSION};
+    stream << entry;
+
+    CCoinJoinEntry roundtripped;
+    BOOST_CHECK_NO_THROW(stream >> roundtripped);
+    BOOST_CHECK_EQUAL(roundtripped.vecTxDSIn.size(), COINJOIN_ENTRY_MAX_SIZE);
+    BOOST_CHECK_EQUAL(roundtripped.vecTxOut.size(), COINJOIN_ENTRY_MAX_SIZE);
+}
+
 BOOST_AUTO_TEST_CASE(queue_timeout_bounds)
 {
     CCoinJoinQueue dsq;