Fix CVE-2025-64438

This commit fixes CVE-2025-64438 by limiting the number of accepted GAPs that
can be stored in the internal set of `WriterProxy`.

This is an squashed commit of a privately reviewed branch.

Signed-off-by: Miguel Company <[email protected]>
Reviewed-by: cferreiragonz <[email protected]>

Author: MiguelCompany

src/cpp/rtps/reader/StatefulReader.cpp

@@ -929,47 +929,23 @@ bool StatefulReader::process_gap_msg(
 
     if (acceptMsgFrom(writerGUID, &pWP) && pWP)
     {
-        // TODO (Miguel C): Refactor this inside WriterProxy
-        SequenceNumber_t auxSN;
-        SequenceNumber_t finalSN = gapList.base();
         History::const_iterator history_iterator = history_->changesBegin();
-        for (auxSN = gapStart; auxSN < finalSN; auxSN++)
-        {
-            if (pWP->irrelevant_change_set(auxSN))
-            {
-                CacheChange_t* to_remove = nullptr;
-                auto ret_iterator = find_cache_in_fragmented_process(auxSN, pWP->guid(), to_remove, history_iterator);
-                if (to_remove != nullptr)
-                {
-                    // we called the History version to avoid callbacks
-                    history_iterator = history_->History::remove_change_nts(ret_iterator);
-                }
-                else if (ret_iterator != history_->changesEnd())
-                {
-                    history_iterator = ret_iterator;
-                }
-            }
-        }
-
-        gapList.for_each(
-            [&](SequenceNumber_t it)
-            {
-                if (pWP->irrelevant_change_set(it))
+        auto remove_fn = [this, &writerGUID, &history_iterator](const SequenceNumber_t& seq)
                 {
                     CacheChange_t* to_remove = nullptr;
-                    auto ret_iterator =
-                    find_cache_in_fragmented_process(auxSN, pWP->guid(), to_remove, history_iterator);
+                    auto ret_iterator = find_cache_in_fragmented_process(seq, writerGUID, to_remove, history_iterator);
                     if (to_remove != nullptr)
                     {
-                        // we called the History version to avoid callbacks
+                        // we call the History version to avoid callbacks
                         history_iterator = history_->History::remove_change_nts(ret_iterator);
                     }
                     else if (ret_iterator != history_->changesEnd())
                     {
                         history_iterator = ret_iterator;
                     }
-                }
-            });
+                };
+
+        pWP->process_gap(gapStart, gapList, remove_fn);
 
         // Maybe now we have to notify user from new CacheChanges.
         NotifyChanges(pWP);

src/cpp/rtps/reader/WriterProxy.h

@@ -20,6 +20,7 @@
 #define FASTDDS_RTPS_READER_WRITERPROXY_H_
 #ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC
 
+#include <algorithm>
 #include <set>
 #include <vector>
 
@@ -143,6 +144,74 @@ class WriterProxy : public RTPSMessageSenderInterface
     bool irrelevant_change_set(
             const SequenceNumber_t& seq_num);
 
+    /**
+     * Process a gap received from the writer.
+     *
+     * @param gap_start  Sequence number as received in the GAP message.
+     * @param gap_list   Sequence number set as received in the GAP message.
+     * @param remove_fn  Function to be called for each irrelevant change found.
+     */
+    template<typename Func>
+    inline void process_gap(
+            const SequenceNumber_t& gap_start,
+            const SequenceNumberSet_t& gap_list,
+            Func&& remove_fn)
+    {
+        // First sequence number that can be considered for GAP processing
+        SequenceNumber_t first_allowed_gap = changes_from_writer_low_mark_ + 1u;
+
+        // Cap start sequence number
+        SequenceNumber_t initial_seq = std::max(gap_start, first_allowed_gap);
+
+        // Default maximum allowed GAP is low mark + 256
+        SequenceNumber_t max_allowed_gap = changes_from_writer_low_mark_ + 256u;
+
+        // Special case when initial_seq is exactly the first allowed GAP
+        if (initial_seq == first_allowed_gap)
+        {
+            max_allowed_gap = gap_list.base() + 256u;
+
+            // Do not exceed max sequence number when known from a heartbeat
+            if (max_sequence_number_ > changes_from_writer_low_mark_)
+            {
+                max_allowed_gap = max_sequence_number_ + 1;
+            }
+        }
+
+        // Early exit if gap_start is beyond allowed range
+        if (gap_start > max_allowed_gap)
+        {
+            return;
+        }
+
+        // Iterate through all sequence numbers in [initial_seq, final_seq)
+        SequenceNumber_t auxSN;
+        SequenceNumber_t finalSN = std::min(gap_list.base(), max_allowed_gap);
+        for (auxSN = initial_seq; auxSN < finalSN; auxSN++)
+        {
+            if (irrelevant_change_set(auxSN))
+            {
+                remove_fn(auxSN);
+            }
+        }
+
+        // Early exit if the entire gap_list is beyond allowed range
+        if (gap_list.base() > max_allowed_gap)
+        {
+            return;
+        }
+
+        // Iterate through all sequence numbers in the gap_list
+        gap_list.for_each(
+            [&](SequenceNumber_t it)
+            {
+                if ((it < max_allowed_gap) && irrelevant_change_set(it))
+                {
+                    remove_fn(it);
+                }
+            });
+    }
+
     /**
      * Check if this proxy has any missing change.
      * @return true when there is at least one missing change on this proxy.

test/blackbox/common/BlackboxTestsTransportUDP.cpp

@@ -808,6 +808,89 @@ TEST(TransportUDP, KeyOnlyBigPayloadIgnored_BestEffort)
     KeyOnlyBigPayloadIgnored(writer, reader);
 }
 
+// Regression test for redmine issue #23829
+TEST(TransportUDP, MaliciousGapBigRange)
+{
+    // Force using UDP transport
+    auto udp_transport = std::make_shared<UDPv4TransportDescriptor>();
+
+    PubSubWriter<UnboundedHelloWorldPubSubType> writer(TEST_TOPIC_NAME);
+    PubSubReader<UnboundedHelloWorldPubSubType> reader(TEST_TOPIC_NAME);
+
+    struct MaliciousGapBigRange
+    {
+        std::array<char, 4> rtps_id{ {'R', 'T', 'P', 'S'} };
+        std::array<uint8_t, 2> protocol_version{ {2, 3} };
+        std::array<uint8_t, 2> vendor_id{ {0x01, 0x0F} };
+        GuidPrefix_t sender_prefix{};
+
+        struct GapSubMsg
+        {
+            uint8_t submessage_id = 0x08;
+#if FASTDDS_IS_BIG_ENDIAN_TARGET
+            uint8_t flags = 0x00;
+#else
+            uint8_t flags = 0x01;
+#endif  // FASTDDS_IS_BIG_ENDIAN_TARGET
+            uint16_t octets_to_next_header = 28;
+            EntityId_t reader_id{};
+            EntityId_t writer_id{};
+            SequenceNumber_t gap_start{ 10u };
+            SequenceNumber_t gap_list_base{ std::numeric_limits<uint32_t>::max() };
+            uint32_t num_longs_bitmap = 0;
+        } gap;
+    };
+
+    UDPMessageSender fake_msg_sender;
+
+    // Set common QoS
+    reader.disable_builtin_transport().add_user_transport_to_pparams(udp_transport)
+        .history_depth(10).reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS);
+    writer.history_depth(10).reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS);
+
+    // Set custom reader locator so we can send malicious data to a known location
+    Locator_t reader_locator;
+    ASSERT_TRUE(IPLocator::setIPv4(reader_locator, "127.0.0.1"));
+    reader_locator.port = 7000;
+    reader.add_to_unicast_locator_list("127.0.0.1", 7000);
+
+    // Initialize and wait for discovery
+    reader.init();
+    ASSERT_TRUE(reader.isInitialized());
+    writer.init();
+    ASSERT_TRUE(writer.isInitialized());
+
+    reader.wait_discovery();
+    writer.wait_discovery();
+
+    // Send 1 sample and wait for reception
+    auto data = default_unbounded_helloworld_data_generator(1);
+    reader.startReception(data);
+    writer.send(data);
+    ASSERT_TRUE(data.empty());
+    reader.block_for_all();
+
+    // Send malicious data
+    {
+        auto writer_guid = writer.datawriter_guid();
+
+        MaliciousGapBigRange malicious_packet{};
+        malicious_packet.sender_prefix = writer_guid.guidPrefix;
+        malicious_packet.gap.writer_id = writer_guid.entityId;
+        malicious_packet.gap.reader_id = reader.datareader_guid().entityId;
+
+        CDRMessage_t msg(0);
+        uint32_t msg_len = static_cast<uint32_t>(sizeof(malicious_packet));
+        msg.init(reinterpret_cast<octet*>(&malicious_packet), msg_len);
+        msg.length = msg_len;
+        msg.pos = msg_len;
+        fake_msg_sender.send(msg, reader_locator);
+    }
+
+    // Block for some time to let the message be processed
+    std::this_thread::sleep_for(std::chrono::milliseconds(500));
+}
+
 // Regression test for redmine issue #23830
 TEST(TransportUDP, MaliciousDataFragUnalignedSizes)
 {