Skip to content

Commit 42e0d08

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 f59b2c2 commit 42e0d08

22 files changed

Lines changed: 1027 additions & 123 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
@@ -2134,6 +2134,33 @@ ReturnCode_t DataWriterImpl::check_qos(
21342134
<< "'" << qos.resource_limits().max_samples_per_instance << "'.");
21352135
return RETCODE_INCONSISTENT_POLICY;
21362136
}
2137+
// Check for nanoseconds in all duration policies
2138+
if (!utils::is_duration_consistent(qos.deadline().period))
2139+
{
2140+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Deadline period is not consistent");
2141+
return RETCODE_INCONSISTENT_POLICY;
2142+
}
2143+
if (!utils::is_duration_consistent(qos.lifespan().duration))
2144+
{
2145+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Lifespan duration is not consistent");
2146+
return RETCODE_INCONSISTENT_POLICY;
2147+
}
2148+
if (!utils::is_duration_consistent(qos.liveliness().lease_duration))
2149+
{
2150+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Liveliness lease duration is not consistent");
2151+
return RETCODE_INCONSISTENT_POLICY;
2152+
}
2153+
if (!utils::is_duration_consistent(qos.liveliness().announcement_period))
2154+
{
2155+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Liveliness announcement period is not consistent");
2156+
return RETCODE_INCONSISTENT_POLICY;
2157+
}
2158+
if (qos.reliability().kind == RELIABLE_RELIABILITY_QOS &&
2159+
!utils::is_duration_consistent(qos.reliability().max_blocking_time, false))
2160+
{
2161+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Reliability max blocking time is not consistent");
2162+
return RETCODE_INCONSISTENT_POLICY;
2163+
}
21372164
return RETCODE_OK;
21382165
}
21392166

src/cpp/fastdds/subscriber/DataReaderImpl.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1627,6 +1627,33 @@ ReturnCode_t DataReaderImpl::check_qos(
16271627
<< "'" << qos.resource_limits().max_samples_per_instance << "'.");
16281628
return RETCODE_INCONSISTENT_POLICY;
16291629
}
1630+
// Check for nanoseconds in all duration policies
1631+
if (!utils::is_duration_consistent(qos.deadline().period))
1632+
{
1633+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Deadline period is not consistent");
1634+
return RETCODE_INCONSISTENT_POLICY;
1635+
}
1636+
if (!utils::is_duration_consistent(qos.lifespan().duration))
1637+
{
1638+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Lifespan duration is not consistent");
1639+
return RETCODE_INCONSISTENT_POLICY;
1640+
}
1641+
if (!utils::is_duration_consistent(qos.liveliness().lease_duration))
1642+
{
1643+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Liveliness lease duration is not consistent");
1644+
return RETCODE_INCONSISTENT_POLICY;
1645+
}
1646+
if (!utils::is_duration_consistent(qos.liveliness().announcement_period))
1647+
{
1648+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Liveliness announcement period is not consistent");
1649+
return RETCODE_INCONSISTENT_POLICY;
1650+
}
1651+
if (qos.reliability().kind == RELIABLE_RELIABILITY_QOS &&
1652+
!utils::is_duration_consistent(qos.reliability().max_blocking_time, false))
1653+
{
1654+
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "Reliability max blocking time is not consistent");
1655+
return RETCODE_INCONSISTENT_POLICY;
1656+
}
16301657
return RETCODE_OK;
16311658
}
16321659

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

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

6491-
vector_t->resize(sequence_length);
6491+
uint32_t bounded_length = sequence_length;
6492+
uint32_t max_bound = type->get_descriptor().bound().at(0);
6493+
if (max_bound != static_cast<uint32_t>(LENGTH_UNLIMITED) && sequence_length > max_bound)
6494+
{
6495+
bounded_length = max_bound;
6496+
EPROSIMA_LOG_WARNING(DYN_TYPES, "Sequence length "
6497+
<< sequence_length << " exceeds the maximum bound of " << max_bound
6498+
<< ". Only " << bounded_length << " elements will be deserialized.");
6499+
}
6500+
6501+
vector_t->resize(bounded_length);
64926502

64936503
for (size_t pos = 0; pos < vector_t->size(); ++pos)
64946504
{
@@ -6500,13 +6510,26 @@ bool DynamicDataImpl::deserialize(
65006510
}
65016511

65026512
uint32_t count {0};
6503-
while (static_cast<uint32_t>(cdr.get_current_position() - offset) < dheader &&
6504-
count < sequence_length)
6513+
while ((static_cast<uint32_t>(cdr.get_current_position() - offset) < dheader) &&
6514+
(count < bounded_length))
65056515
{
65066516
cdr.deserialize(vector_t->data()[count]);
65076517
++count;
65086518
}
65096519

6520+
if (bounded_length < sequence_length)
6521+
{
6522+
auto tmp_data = traits<DynamicData>::narrow<DynamicDataImpl>(
6523+
DynamicDataFactory::get_instance()->create_data(element_type));
6524+
// Skip remaining elements if sequence length exceeds maximum bound
6525+
while ((static_cast<uint32_t>(cdr.get_current_position() - offset) < dheader) &&
6526+
(count < sequence_length))
6527+
{
6528+
cdr.deserialize(tmp_data);
6529+
++count;
6530+
}
6531+
}
6532+
65106533
if (static_cast<uint32_t>(cdr.get_current_position() - offset) != dheader)
65116534
{
65126535
throw fastcdr::exception::BadParamException(
@@ -6531,7 +6554,18 @@ bool DynamicDataImpl::deserialize(
65316554
type->get_descriptor().element_type()));
65326555
try
65336556
{
6534-
vector_t->resize(sequence_length);
6557+
uint32_t bounded_length = sequence_length;
6558+
uint32_t max_bound = type->get_descriptor().bound().at(0);
6559+
if (max_bound != static_cast<uint32_t>(LENGTH_UNLIMITED) && sequence_length > max_bound)
6560+
{
6561+
bounded_length = max_bound;
6562+
EPROSIMA_LOG_WARNING(DYN_TYPES, "Sequence length "
6563+
<< sequence_length << " exceeds the maximum bound of "
6564+
<< max_bound << ". Only "
6565+
<< bounded_length << " elements will be deserialized.");
6566+
}
6567+
6568+
vector_t->resize(bounded_length);
65356569
for (size_t pos = 0; pos < vector_t->size(); ++pos)
65366570
{
65376571
if (!vector_t->at(pos))
@@ -6541,6 +6575,17 @@ bool DynamicDataImpl::deserialize(
65416575
}
65426576
}
65436577
cdr.deserialize_array(vector_t->data(), vector_t->size());
6578+
6579+
if (bounded_length < sequence_length)
6580+
{
6581+
auto tmp_data = traits<DynamicData>::narrow<DynamicDataImpl>(
6582+
DynamicDataFactory::get_instance()->create_data(element_type));
6583+
// Skip remaining elements if sequence length exceeds maximum bound
6584+
for (uint32_t count = bounded_length; count < sequence_length; ++count)
6585+
{
6586+
cdr.deserialize(tmp_data);
6587+
}
6588+
}
65446589
}
65456590
catch (fastcdr::exception::Exception& ex)
65466591
{

src/cpp/rtps/messages/MessageReceiver.cpp

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

10291029
uint32_t payload_size;
1030-
payload_size = smh->submessageLength - (RTPSMESSAGE_DATA_EXTRA_INLINEQOS_SIZE + octetsToInlineQos + inlineQosSize);
1030+
const uint32_t submsg_no_payload_size = RTPSMESSAGE_DATA_EXTRA_INLINEQOS_SIZE + octetsToInlineQos + inlineQosSize;
1031+
if (smh->submessageLength < submsg_no_payload_size)
1032+
{
1033+
EPROSIMA_LOG_WARNING(RTPS_MSG_IN, IDSTRING "Serialized Payload avoided underflow "
1034+
"(" << smh->submessageLength << "/" << submsg_no_payload_size << ")");
1035+
ch.serializedPayload.data = nullptr;
1036+
ch.inline_qos.data = nullptr;
1037+
return false;
1038+
}
1039+
payload_size = smh->submessageLength - submsg_no_payload_size;
10311040

10321041
// Validations??? XXX TODO
10331042

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)