Skip to content

Commit 104a71d

Browse files
committed
Fix CVE-2025-65016
This commit fixes CVE-2025-65016 by improving the way changes are notified in stateful readers. This is an squashed commit of a privately reviewed branch. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Reviewed-by: cferreiragonz <carlosferreira@eprosima.com>
1 parent 852382f commit 104a71d

File tree

4 files changed

+115
-5
lines changed

4 files changed

+115
-5
lines changed

src/cpp/rtps/reader/StatefulReader.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,9 +1206,7 @@ void StatefulReader::NotifyChanges(
12061206
while (next_seq != c_SequenceNumber_Unknown && next_seq <= aux_ch->sequenceNumber);
12071207
}
12081208
// Ensure correct state of proxy when max_seq is not present in history
1209-
while (c_SequenceNumber_Unknown != prox->next_cache_change_to_be_notified())
1210-
{
1211-
}
1209+
prox->consider_all_notified();
12121210

12131211
// Notify listener if new data is available
12141212
auto listener = getListener();

src/cpp/rtps/reader/WriterProxy.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,18 @@ SequenceNumber_t WriterProxy::next_cache_change_to_be_notified()
486486
return SequenceNumber_t::unknown();
487487
}
488488

489+
void WriterProxy::consider_all_notified()
490+
{
491+
#ifdef SHOULD_DEBUG_LINUX
492+
assert(get_mutex_owner() == get_thread_id());
493+
#endif // SHOULD_DEBUG_LINUX
494+
495+
if (last_notified_ < changes_from_writer_low_mark_)
496+
{
497+
last_notified_ = changes_from_writer_low_mark_;
498+
}
499+
}
500+
489501
bool WriterProxy::perform_initial_ack_nack()
490502
{
491503
bool ret_value = false;

src/cpp/rtps/reader/WriterProxy.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,14 @@ class WriterProxy : public RTPSMessageSenderInterface
293293
*/
294294
SequenceNumber_t next_cache_change_to_be_notified();
295295

296+
/**
297+
* @brief Marks all available sequence numbers as notified.
298+
*
299+
* Calling this function is the equivalent to calling next_cache_change_to_be_notified
300+
* in a loop until it returns an invalid SequenceNumber_t.
301+
*/
302+
void consider_all_notified();
303+
296304
/**
297305
* Checks whether a cache change was already received from this proxy.
298306
* @param[in] seq_num Sequence number of the cache change to check.

test/blackbox/common/BlackboxTestsTransportUDP.cpp

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -838,14 +838,15 @@ TEST(TransportUDP, MaliciousGapBigRange)
838838
SequenceNumber_t gap_start{ 10u };
839839
SequenceNumber_t gap_list_base{ std::numeric_limits<uint32_t>::max() };
840840
uint32_t num_longs_bitmap = 0;
841-
} gap;
841+
}
842+
gap;
842843
};
843844

844845
UDPMessageSender fake_msg_sender;
845846

846847
// Set common QoS
847848
reader.disable_builtin_transport().add_user_transport_to_pparams(udp_transport)
848-
.history_depth(10).reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS);
849+
.history_depth(10).reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS);
849850
writer.history_depth(10).reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS);
850851

851852
// Set custom reader locator so we can send malicious data to a known location
@@ -988,6 +989,97 @@ TEST(TransportUDP, MaliciousDataFragUnalignedSizes)
988989
reader.block_for_all();
989990
}
990991

992+
// Regression test for redmine issue #23917
993+
TEST(TransportUDP, MaliciousHeartbeatBigStart)
994+
{
995+
// Force using UDP transport
996+
auto udp_transport = std::make_shared<UDPv4TransportDescriptor>();
997+
998+
PubSubWriter<UnboundedHelloWorldPubSubType> writer(TEST_TOPIC_NAME);
999+
PubSubReader<UnboundedHelloWorldPubSubType> reader(TEST_TOPIC_NAME);
1000+
1001+
struct MaliciousHeartbeatBigStart
1002+
{
1003+
std::array<char, 4> rtps_id{ {'R', 'T', 'P', 'S'} };
1004+
std::array<uint8_t, 2> protocol_version{ {2, 3} };
1005+
std::array<uint8_t, 2> vendor_id{ {0x01, 0x0F} };
1006+
GuidPrefix_t sender_prefix{};
1007+
1008+
struct HeartbeatSubMsg
1009+
{
1010+
uint8_t submessage_id = 0x07;
1011+
#if FASTDDS_IS_BIG_ENDIAN_TARGET
1012+
uint8_t flags = 0x00;
1013+
#else
1014+
uint8_t flags = 0x01;
1015+
#endif // FASTDDS_IS_BIG_ENDIAN_TARGET
1016+
uint16_t octets_to_next_header = 28;
1017+
EntityId_t reader_id{};
1018+
EntityId_t writer_id{};
1019+
SequenceNumber_t first_sn{ 1, 0u };
1020+
SequenceNumber_t last_sn{ 1, 1u };
1021+
uint32_t hb_count = 0x7fffffffu;
1022+
}
1023+
hb;
1024+
};
1025+
1026+
UDPMessageSender fake_msg_sender;
1027+
1028+
// Set common QoS
1029+
reader.disable_builtin_transport().add_user_transport_to_pparams(udp_transport)
1030+
.history_depth(10).reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS);
1031+
writer.history_depth(10).reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS);
1032+
1033+
// Set custom reader locator so we can send malicious data to a known location
1034+
Locator_t reader_locator;
1035+
ASSERT_TRUE(IPLocator::setIPv4(reader_locator, "127.0.0.1"));
1036+
reader_locator.port = 7000;
1037+
reader.add_to_unicast_locator_list("127.0.0.1", 7000);
1038+
1039+
// Initialize and wait for discovery
1040+
reader.init();
1041+
ASSERT_TRUE(reader.isInitialized());
1042+
writer.init();
1043+
ASSERT_TRUE(writer.isInitialized());
1044+
1045+
reader.wait_discovery();
1046+
writer.wait_discovery();
1047+
1048+
// Send 10 samples
1049+
auto data = default_unbounded_helloworld_data_generator(10);
1050+
auto expected_data = data;
1051+
writer.send(data);
1052+
ASSERT_TRUE(data.empty());
1053+
1054+
auto start = std::chrono::steady_clock::now();
1055+
1056+
// Send malicious heartbeat before taking samples
1057+
{
1058+
auto writer_guid = writer.datawriter_guid();
1059+
1060+
MaliciousHeartbeatBigStart malicious_packet{};
1061+
malicious_packet.sender_prefix = writer_guid.guidPrefix;
1062+
malicious_packet.hb.writer_id = writer_guid.entityId;
1063+
malicious_packet.hb.reader_id = reader.datareader_guid().entityId;
1064+
1065+
CDRMessage_t msg(0);
1066+
uint32_t msg_len = static_cast<uint32_t>(sizeof(malicious_packet));
1067+
msg.init(reinterpret_cast<octet*>(&malicious_packet), msg_len);
1068+
msg.length = msg_len;
1069+
msg.pos = msg_len;
1070+
fake_msg_sender.send(msg, reader_locator);
1071+
}
1072+
1073+
// Start reception of expected data
1074+
reader.startReception(expected_data);
1075+
EXPECT_EQ(reader.block_for_all(std::chrono::seconds(10)), expected_data.size());
1076+
reader.destroy();
1077+
1078+
// Ensure that the test does not take too long
1079+
auto end = std::chrono::steady_clock::now();
1080+
EXPECT_LT(end - start, std::chrono::seconds(15));
1081+
}
1082+
9911083
// Test for ==operator UDPTransportDescriptor is not required as it is an abstract class and in UDPv4 is same method
9921084
// Test for copy UDPTransportDescriptor is not required as it is an abstract class and in UDPv4 is same method
9931085

0 commit comments

Comments
 (0)