Skip to content

Commit 7ab89d6

Browse files
Fix several CVEs
* Fix [CVE-2025-63829] by avoiding the infinite loop in `Time_t::fraction()`, and checking for consistency in the nanosecond values of QoS. * Fix [CVE-2025-65865] by checking values before substraction in `proc_Submsg_DataFrag`. * Fix [CVE-2026-45093] with a refactor of `RTCPMessageManager`. * Fix [CVE-2026-45092] by applying limits to the size of `SecurityManager::discovered_participants_`. * Fix [CVE-2026-45097] by checking bounds when deserializing sequences in `DynamicDataImpl`. This is a squashed commit of a privately reviewed branch. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: Zakaria Talbi Lalmi <zakariatalbi@eprosima.com> Co-authored-by: Zakaria Talbi Lalmi <zakariatalbi@eprosima.com> Reviewed-by: cferreiragonz <carlosferreira@eprosima.com>
1 parent 82bfda2 commit 7ab89d6

22 files changed

Lines changed: 1026 additions & 124 deletions

src/cpp/fastdds/core/Time_t.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,16 @@ void Time_t::fraction(
5757

5858
uint32_t Time_t::fraction() const
5959
{
60-
uint32_t fraction = (nanosec == 0xffffffff)
61-
? 0xffffffff
62-
: nano_to_frac(nanosec);
60+
constexpr uint32_t infinite_fraction = 0xffffffff;
61+
constexpr uint32_t max_fraction = infinite_fraction - 1;
6362

64-
if (fraction != 0xffffffff)
63+
uint32_t fraction = infinite_fraction;
64+
uint64_t nanosec_64 = nanosec;
65+
if (nanosec_64 < C_NANOSECONDS_PER_SEC)
6566
{
67+
fraction = nano_to_frac(nanosec);
6668
uint32_t nano_check = frac_to_nano(fraction);
67-
while (nano_check != nanosec)
69+
while ((nano_check != nanosec) && (fraction < max_fraction))
6870
{
6971
nano_check = frac_to_nano(++fraction);
7072
}

src/cpp/fastdds/core/policy/QosPolicyUtils.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919

2020
#include <fastdds/core/policy/QosPolicyUtils.hpp>
21+
#include <fastdds/dds/core/Time_t.hpp>
2122

2223
#include <utils/Host.hpp>
2324

@@ -38,6 +39,23 @@ uint64_t default_domain_id()
3839
return id;
3940
}
4041

42+
bool is_duration_consistent(
43+
const Duration_t& duration,
44+
bool allow_infinite)
45+
{
46+
if (duration.is_infinite())
47+
{
48+
return allow_infinite;
49+
}
50+
51+
if (duration.seconds < 0 || duration.nanosec >= 1000000000)
52+
{
53+
return false;
54+
}
55+
56+
return true;
57+
}
58+
4159
} // namespace utils
4260
} // namespace dds
4361
} // namespace fastdds

src/cpp/fastdds/core/policy/QosPolicyUtils.hpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
#ifndef _FASTDDS_DDS_QOS_QOSPOLICYUTILS_HPP_
2121
#define _FASTDDS_DDS_QOS_QOSPOLICYUTILS_HPP_
2222

23-
#include <stdint.h>
23+
#include <cstdint>
24+
25+
#include <fastdds/dds/core/Time_t.hpp>
2426

2527
namespace eprosima {
2628
namespace fastdds {
@@ -30,6 +32,18 @@ namespace utils {
3032
// Compute the default DataSharing domain ID
3133
uint64_t default_domain_id();
3234

35+
/**
36+
* @brief Checks if a Duration_t is consistent, i.e. seconds is non-negative, and nanosec is less than 1 second.
37+
*
38+
* @param duration Duration to check.
39+
* @param allow_infinite Whether to allow infinite duration.
40+
*
41+
* @return true if the duration is consistent, false otherwise.
42+
*/
43+
bool is_duration_consistent(
44+
const Duration_t& duration,
45+
bool allow_infinite = true);
46+
3347
} // namespace utils
3448
} // namespace dds
3549
} // namespace fastdds

src/cpp/fastdds/publisher/DataWriterImpl.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2078,6 +2078,33 @@ ReturnCode_t DataWriterImpl::check_qos(
20782078
<< "'. Consistency rule: depth <= max_samples_per_instance."
20792079
<< " Effectively using max_samples_per_instance as depth.");
20802080
}
2081+
// Check for nanoseconds in all duration policies
2082+
if (!utils::is_duration_consistent(qos.deadline().period))
2083+
{
2084+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Deadline period is not consistent");
2085+
return RETCODE_INCONSISTENT_POLICY;
2086+
}
2087+
if (!utils::is_duration_consistent(qos.lifespan().duration))
2088+
{
2089+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Lifespan duration is not consistent");
2090+
return RETCODE_INCONSISTENT_POLICY;
2091+
}
2092+
if (!utils::is_duration_consistent(qos.liveliness().lease_duration))
2093+
{
2094+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Liveliness lease duration is not consistent");
2095+
return RETCODE_INCONSISTENT_POLICY;
2096+
}
2097+
if (!utils::is_duration_consistent(qos.liveliness().announcement_period))
2098+
{
2099+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Liveliness announcement period is not consistent");
2100+
return RETCODE_INCONSISTENT_POLICY;
2101+
}
2102+
if (qos.reliability().kind == RELIABLE_RELIABILITY_QOS &&
2103+
!utils::is_duration_consistent(qos.reliability().max_blocking_time, false))
2104+
{
2105+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Reliability max blocking time is not consistent");
2106+
return RETCODE_INCONSISTENT_POLICY;
2107+
}
20812108
return RETCODE_OK;
20822109
}
20832110

src/cpp/fastdds/subscriber/DataReaderImpl.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,6 +1621,33 @@ ReturnCode_t DataReaderImpl::check_qos(
16211621
"'. Consistency rule: depth <= max_samples_per_instance." <<
16221622
" Effectively using max_samples_per_instance as depth.");
16231623
}
1624+
// Check for nanoseconds in all duration policies
1625+
if (!utils::is_duration_consistent(qos.deadline().period))
1626+
{
1627+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Deadline period is not consistent");
1628+
return RETCODE_INCONSISTENT_POLICY;
1629+
}
1630+
if (!utils::is_duration_consistent(qos.lifespan().duration))
1631+
{
1632+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Lifespan duration is not consistent");
1633+
return RETCODE_INCONSISTENT_POLICY;
1634+
}
1635+
if (!utils::is_duration_consistent(qos.liveliness().lease_duration))
1636+
{
1637+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Liveliness lease duration is not consistent");
1638+
return RETCODE_INCONSISTENT_POLICY;
1639+
}
1640+
if (!utils::is_duration_consistent(qos.liveliness().announcement_period))
1641+
{
1642+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Liveliness announcement period is not consistent");
1643+
return RETCODE_INCONSISTENT_POLICY;
1644+
}
1645+
if (qos.reliability().kind == RELIABLE_RELIABILITY_QOS &&
1646+
!utils::is_duration_consistent(qos.reliability().max_blocking_time, false))
1647+
{
1648+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Reliability max blocking time is not consistent");
1649+
return RETCODE_INCONSISTENT_POLICY;
1650+
}
16241651
return RETCODE_OK;
16251652
}
16261653

src/cpp/fastdds/xtypes/dynamic_types/DynamicDataImpl.cpp

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6315,7 +6315,17 @@ bool DynamicDataImpl::deserialize(
63156315
get_enclosing_type(traits<DynamicType>::narrow<DynamicTypeImpl>(
63166316
type->get_descriptor().element_type()));
63176317

6318-
vector_t->resize(sequence_length);
6318+
uint32_t bounded_length = sequence_length;
6319+
uint32_t max_bound = type->get_descriptor().bound().at(0);
6320+
if (max_bound != static_cast<uint32_t>(LENGTH_UNLIMITED) && sequence_length > max_bound)
6321+
{
6322+
bounded_length = max_bound;
6323+
EPROSIMA_LOG_WARNING(DYN_TYPES, "Sequence length "
6324+
<< sequence_length << " exceeds the maximum bound of " << max_bound
6325+
<< ". Only " << bounded_length << " elements will be deserialized.");
6326+
}
6327+
6328+
vector_t->resize(bounded_length);
63196329

63206330
for (size_t pos = 0; pos < vector_t->size(); ++pos)
63216331
{
@@ -6327,13 +6337,26 @@ bool DynamicDataImpl::deserialize(
63276337
}
63286338

63296339
uint32_t count {0};
6330-
while (static_cast<uint32_t>(cdr.get_current_position() - offset) < dheader &&
6331-
count < sequence_length)
6340+
while ((static_cast<uint32_t>(cdr.get_current_position() - offset) < dheader) &&
6341+
(count < bounded_length))
63326342
{
63336343
cdr.deserialize(vector_t->data()[count]);
63346344
++count;
63356345
}
63366346

6347+
if (bounded_length < sequence_length)
6348+
{
6349+
auto tmp_data = traits<DynamicData>::narrow<DynamicDataImpl>(
6350+
DynamicDataFactory::get_instance()->create_data(element_type));
6351+
// Skip remaining elements if sequence length exceeds maximum bound
6352+
while ((static_cast<uint32_t>(cdr.get_current_position() - offset) < dheader) &&
6353+
(count < sequence_length))
6354+
{
6355+
cdr.deserialize(tmp_data);
6356+
++count;
6357+
}
6358+
}
6359+
63376360
if (static_cast<uint32_t>(cdr.get_current_position() - offset) != dheader)
63386361
{
63396362
throw fastcdr::exception::BadParamException(
@@ -6358,7 +6381,18 @@ bool DynamicDataImpl::deserialize(
63586381
type->get_descriptor().element_type()));
63596382
try
63606383
{
6361-
vector_t->resize(sequence_length);
6384+
uint32_t bounded_length = sequence_length;
6385+
uint32_t max_bound = type->get_descriptor().bound().at(0);
6386+
if (max_bound != static_cast<uint32_t>(LENGTH_UNLIMITED) && sequence_length > max_bound)
6387+
{
6388+
bounded_length = max_bound;
6389+
EPROSIMA_LOG_WARNING(DYN_TYPES, "Sequence length "
6390+
<< sequence_length << " exceeds the maximum bound of "
6391+
<< max_bound << ". Only "
6392+
<< bounded_length << " elements will be deserialized.");
6393+
}
6394+
6395+
vector_t->resize(bounded_length);
63626396
for (size_t pos = 0; pos < vector_t->size(); ++pos)
63636397
{
63646398
if (!vector_t->at(pos))
@@ -6368,6 +6402,17 @@ bool DynamicDataImpl::deserialize(
63686402
}
63696403
}
63706404
cdr.deserialize_array(vector_t->data(), vector_t->size());
6405+
6406+
if (bounded_length < sequence_length)
6407+
{
6408+
auto tmp_data = traits<DynamicData>::narrow<DynamicDataImpl>(
6409+
DynamicDataFactory::get_instance()->create_data(element_type));
6410+
// Skip remaining elements if sequence length exceeds maximum bound
6411+
for (uint32_t count = bounded_length; count < sequence_length; ++count)
6412+
{
6413+
cdr.deserialize(tmp_data);
6414+
}
6415+
}
63716416
}
63726417
catch (fastcdr::exception::Exception& ex)
63736418
{

src/cpp/rtps/messages/MessageReceiver.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,16 @@ bool MessageReceiver::proc_Submsg_DataFrag(
10421042
}
10431043

10441044
uint32_t payload_size;
1045-
payload_size = smh->submessageLength - (RTPSMESSAGE_DATA_EXTRA_INLINEQOS_SIZE + octetsToInlineQos + inlineQosSize);
1045+
const uint32_t submsg_no_payload_size = RTPSMESSAGE_DATA_EXTRA_INLINEQOS_SIZE + octetsToInlineQos + inlineQosSize;
1046+
if (smh->submessageLength < submsg_no_payload_size)
1047+
{
1048+
EPROSIMA_LOG_WARNING(RTPS_MSG_IN, IDSTRING "Serialized Payload avoided underflow "
1049+
"(" << smh->submessageLength << "/" << submsg_no_payload_size << ")");
1050+
ch.serializedPayload.data = nullptr;
1051+
ch.inline_qos.data = nullptr;
1052+
return false;
1053+
}
1054+
payload_size = smh->submessageLength - submsg_no_payload_size;
10461055

10471056
// Validations??? XXX TODO
10481057

src/cpp/rtps/security/SecurityManager.cpp

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ SecurityManager::SecurityManager(
9595
, participant_(participant)
9696
, factory_(plugin_factory)
9797
, domain_id_(0)
98+
, max_discovered_participants_(pattr.allocation.participants.maximum)
9899
, auth_last_sequence_number_(1)
99100
, crypto_last_sequence_number_(1)
100101
, temp_reader_proxies_({
@@ -618,28 +619,48 @@ bool SecurityManager::discovered_participant(
618619
AuthenticationStatus auth_status = AUTHENTICATION_INIT;
619620

620621
// Create or find information
621-
bool undiscovered = false;
622+
bool newly_discovered = false;
622623
DiscoveredParticipantInfo::AuthUniquePtr remote_participant_info;
623624
// Use the information from the collection
624625
const ParticipantProxyData* remote_participant_data = nullptr;
625626
{
626627
std::lock_guard<shared_mutex> _(mutex_);
627628

628-
auto map_ret = discovered_participants_.insert(
629-
std::make_pair(
629+
auto dp_it = discovered_participants_.lower_bound(participant_data.guid);
630+
if ((dp_it != discovered_participants_.end()) && (dp_it->first == participant_data.guid))
631+
{
632+
// Already exists, use the information from the collection
633+
remote_participant_info = dp_it->second->get_auth();
634+
remote_participant_data = &dp_it->second->participant_data();
635+
}
636+
else if (discovered_participants_.size() < max_discovered_participants_)
637+
{
638+
// Create new element, because it is not discovered yet
639+
auto map_ret = discovered_participants_.emplace_hint(
640+
dp_it,
630641
participant_data.guid,
631642
std::unique_ptr<DiscoveredParticipantInfo>(
632-
new DiscoveredParticipantInfo(
633-
auth_status,
634-
participant_data))));
643+
new DiscoveredParticipantInfo(auth_status, participant_data)));
635644

636-
undiscovered = map_ret.second;
637-
remote_participant_info = map_ret.first->second->get_auth();
638-
remote_participant_data = &map_ret.first->second->participant_data();
645+
// New element, so mark as newly_discovered
646+
newly_discovered = true;
647+
// Use the recently created information from the collection
648+
remote_participant_info = map_ret->second->get_auth();
649+
remote_participant_data = &map_ret->second->participant_data();
650+
}
651+
else
652+
{
653+
// Using info level to avoid clogging the logs, since this could be triggered by an attacker flooding the
654+
// network with fake participants.
655+
EPROSIMA_LOG_INFO(SECURITY,
656+
"Maximum number of discovered participants reached. Ignoring participant "
657+
<< participant_data.guid);
658+
return false;
659+
}
639660
}
640661

641662
bool notify_part_authorized = false;
642-
if (undiscovered && remote_participant_info && remote_participant_data != nullptr)
663+
if (newly_discovered && remote_participant_info && remote_participant_data != nullptr)
643664
{
644665
// Configure the timed event but do not start it
645666
const GUID_t guid = remote_participant_data->guid;

src/cpp/rtps/security/SecurityManager.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,9 @@ class SecurityManager : private WriterListener
834834
// synchronization
835835
std::map<GUID_t, std::unique_ptr<DiscoveredParticipantInfo>> discovered_participants_;
836836

837+
// Maximum allowed size for the collection of discovered participants.
838+
const size_t max_discovered_participants_;
839+
837840
GUID_t auth_source_guid;
838841

839842
/**

0 commit comments

Comments
 (0)