Merge #14685: fix a deserialization overflow edge case

b08af10fb299dc3fdcd1f022619fb112c72e5d8e disallow oversized CBlockHeaderAndShortTxIDs (Kaz Wesley)
6bed4b374daf26233e96fa7863d4324a5bfa99c2 fix a deserialization overflow edge case (Kaz Wesley)
051faf7e9d4e32142f95f7adb31d2f53f656cb66 add a test demonstrating an overflow in a deserialization edge case (Kaz Wesley)

Pull request description:

  A specially-constructed BlockTransactionsRequest can cause `offset` to wrap in deserialization. In the current code, there is not any way this could be dangerous; but disallowing it reduces the potential for future surprises.

Tree-SHA512: 1aaf7636e0801a905ed8807d0d1762132ac8b4421a600c35fb6d5e5033c6bfb587d8668cd9f48c7a08a2ae793a677b7649661e3ae248ab4f8499ab7b6ede483c

1 files changed, 45 insertions, 0 deletions

diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp
index 5131fe8235..309b8d2d06 100644
--- a/src/test/blockencodings_tests.cpp
+++ b/src/test/blockencodings_tests.cpp
@@ -344,4 +344,49 @@ BOOST_AUTO_TEST_CASE(TransactionsRequestSerializationTest) {
     BOOST_CHECK_EQUAL(req1.indexes[3], req2.indexes[3]);
 }
 
+BOOST_AUTO_TEST_CASE(TransactionsRequestDeserializationMaxTest) {
+    // Check that the highest legal index is decoded correctly
+    BlockTransactionsRequest req0;
+    req0.blockhash = InsecureRand256();
+    req0.indexes.resize(1);
+    req0.indexes[0] = 0xffff;
+    CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
+    stream << req0;
+
+    BlockTransactionsRequest req1;
+    stream >> req1;
+    BOOST_CHECK_EQUAL(req0.indexes.size(), req1.indexes.size());
+    BOOST_CHECK_EQUAL(req0.indexes[0], req1.indexes[0]);
+}
+
+BOOST_AUTO_TEST_CASE(TransactionsRequestDeserializationOverflowTest) {
+    // Any set of index deltas that starts with N values that sum to (0x10000 - N)
+    // causes the edge-case overflow that was originally not checked for. Such
+    // a request cannot be created by serializing a real BlockTransactionsRequest
+    // due to the overflow, so here we'll serialize from raw deltas.
+    BlockTransactionsRequest req0;
+    req0.blockhash = InsecureRand256();
+    req0.indexes.resize(3);
+    req0.indexes[0] = 0x7000;
+    req0.indexes[1] = 0x10000 - 0x7000 - 2;
+    req0.indexes[2] = 0;
+    CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
+    stream << req0.blockhash;
+    WriteCompactSize(stream, req0.indexes.size());
+    WriteCompactSize(stream, req0.indexes[0]);
+    WriteCompactSize(stream, req0.indexes[1]);
+    WriteCompactSize(stream, req0.indexes[2]);
+
+    BlockTransactionsRequest req1;
+    try {
+        stream >> req1;
+        // before patch: deserialize above succeeds and this check fails, demonstrating the overflow
+        BOOST_CHECK(req1.indexes[1] < req1.indexes[2]);
+        // this shouldn't be reachable before or after patch
+        BOOST_CHECK(0);
+    } catch(std::ios_base::failure &) {
+        // deserialize should fail
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()