diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index e17bd14bbc..6c42159102 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -93,6 +93,7 @@ test.overlay > test.toplevel test.overlay > test.unit_test test.overlay > xrpl.basics test.overlay > xrpld.app +test.overlay > xrpld.nodestore test.overlay > xrpld.overlay test.overlay > xrpld.peerfinder test.overlay > xrpld.shamap diff --git a/src/test/overlay/TMGetObjectByHash_test.cpp b/src/test/overlay/TMGetObjectByHash_test.cpp new file mode 100644 index 0000000000..8e74779776 --- /dev/null +++ b/src/test/overlay/TMGetObjectByHash_test.cpp @@ -0,0 +1,211 @@ +#include +#include + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace ripple { +namespace test { + +using namespace jtx; + +/** + * Test for TMGetObjectByHash reply size limiting. + * + * This verifies the fix that limits TMGetObjectByHash replies to + * Tuning::hardMaxReplyNodes to prevent excessive memory usage and + * potential DoS attacks from peers requesting large numbers of objects. + */ +class TMGetObjectByHash_test : public beast::unit_test::suite +{ + using middle_type = boost::beast::tcp_stream; + using stream_type = boost::beast::ssl_stream; + using socket_type = boost::asio::ip::tcp::socket; + using shared_context = std::shared_ptr; + /** + * Test peer that captures sent messages for verification. + */ + class PeerTest : public PeerImp + { + public: + PeerTest( + Application& app, + std::shared_ptr const& slot, + http_request_type&& request, + PublicKey const& publicKey, + ProtocolVersion protocol, + Resource::Consumer consumer, + std::unique_ptr&& stream_ptr, + OverlayImpl& overlay) + : PeerImp( + app, + id_++, + slot, + std::move(request), + publicKey, + protocol, + consumer, + std::move(stream_ptr), + overlay) + { + } + + ~PeerTest() = default; + + void + run() override + { + } + + void + send(std::shared_ptr const& m) override + { + lastSentMessage_ = m; + } + + std::shared_ptr + getLastSentMessage() const + { + return lastSentMessage_; + } + + static void + resetId() + { + id_ = 0; + } + + private: + inline static Peer::id_t id_ = 0; + std::shared_ptr lastSentMessage_; + }; + + shared_context context_{make_SSLContext("")}; + ProtocolVersion protocolVersion_{1, 7}; + + std::shared_ptr + createPeer(jtx::Env& env) + { + auto& overlay = dynamic_cast(env.app().overlay()); + boost::beast::http::request request; + auto stream_ptr = std::make_unique( + socket_type(env.app().getIOService()), *context_); + + beast::IP::Endpoint local( + boost::asio::ip::make_address("172.1.1.1"), 51235); + beast::IP::Endpoint remote( + boost::asio::ip::make_address("172.1.1.2"), 51235); + + PublicKey key(std::get<0>(randomKeyPair(KeyType::ed25519))); + auto consumer = overlay.resourceManager().newInboundEndpoint(remote); + auto [slot, _] = overlay.peerFinder().new_inbound_slot(local, remote); + + auto peer = std::make_shared( + env.app(), + slot, + std::move(request), + key, + protocolVersion_, + consumer, + std::move(stream_ptr), + overlay); + + overlay.add_active(peer); + return peer; + } + + std::shared_ptr + createRequest(size_t const numObjects, Env& env) + { + // Store objects in the NodeStore that will be found during the query + auto& nodeStore = env.app().getNodeStore(); + + // Create and store objects + std::vector hashes; + hashes.reserve(numObjects); + for (int i = 0; i < numObjects; ++i) + { + uint256 hash(ripple::sha512Half(i)); + hashes.push_back(hash); + + Blob data(100, static_cast(i % 256)); + nodeStore.store( + hotLEDGER, + std::move(data), + hash, + nodeStore.earliestLedgerSeq()); + } + + // Create a request with more objects than hardMaxReplyNodes + auto request = std::make_shared(); + request->set_type(protocol::TMGetObjectByHash_ObjectType_otLEDGER); + request->set_query(true); + + for (int i = 0; i < numObjects; ++i) + { + auto object = request->add_objects(); + object->set_hash(hashes[i].data(), hashes[i].size()); + object->set_ledgerseq(i); + } + return request; + } + + /** + * Test that reply is limited to hardMaxReplyNodes when more objects + * are requested than the limit allows. + */ + void + testReplyLimit(size_t const numObjects, int const expectedReplySize) + { + testcase("Reply Limit"); + + Env env(*this); + PeerTest::resetId(); + + auto peer = createPeer(env); + + auto request = createRequest(numObjects, env); + // Call the onMessage handler + peer->onMessage(request); + + // Verify that a reply was sent + auto sentMessage = peer->getLastSentMessage(); + BEAST_EXPECT(sentMessage != nullptr); + + // Parse the reply message + auto const& buffer = + sentMessage->getBuffer(compression::Compressed::Off); + + BEAST_EXPECT(buffer.size() > 6); + // Skip the message header (6 bytes: 4 for size, 2 for type) + protocol::TMGetObjectByHash reply; + BEAST_EXPECT( + reply.ParseFromArray(buffer.data() + 6, buffer.size() - 6) == true); + + // Verify the reply is limited to expectedReplySize + BEAST_EXPECT(reply.objects_size() == expectedReplySize); + } + + void + run() override + { + int const limit = static_cast(Tuning::hardMaxReplyNodes); + testReplyLimit(limit + 1, limit); + testReplyLimit(limit, limit); + testReplyLimit(limit - 1, limit - 1); + } +}; + +BEAST_DEFINE_TESTSUITE(TMGetObjectByHash, overlay, xrpl); + +} // namespace test +} // namespace ripple diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index eef199fa35..8e33de12b2 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1272,8 +1272,8 @@ PeerImp::handleTransaction( { // If we've never been in synch, there's nothing we can do // with a transaction - JLOG(p_journal_.debug()) << "Ignoring incoming transaction: " - << "Need network ledger"; + JLOG(p_journal_.debug()) + << "Ignoring incoming transaction: Need network ledger"; return; } @@ -2551,6 +2551,16 @@ PeerImp::onMessage(std::shared_ptr const& m) newObj.set_ledgerseq(obj.ledgerseq()); // VFALCO NOTE "seq" in the message is obsolete + + // Check if by adding this object, reply has reached its + // limit + if (reply.objects_size() >= Tuning::hardMaxReplyNodes) + { + fee_.update( + Resource::feeModerateBurdenPeer, + " Reply limit reached. Truncating reply."); + break; + } } } }