CVE-2024-30258: Insufficient Information (NVD-CWE-noinfo)
value: FastDDS is a C++ implementation of the DDS (Data Distribution Service) standard of the OMG (Object Management Group). Prior to versions 2.14.1, 2.13.5, 2.10.4, and 2.6.8, when a publisher serves a malformed `RTPS` packet, the subscriber crashes when creating `pthread`. This can remotely crash any Fast-DDS process, potentially leading to a DOS attack. Versions 2.14.1, 2.13.5, 2.10.4, and 2.6.8 contain a patch for the issue.

CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-14T15:22Z
Repository: https://github.com/eProsima/Fast-DDS
Language: C++
Files changed: include/fastdds/rtps/messages/CDRMessage.h, include/fastdds/rtps/messages/CDRMessage.hpp, src/cpp/fastdds/core/policy/ParameterSerializer.hpp, test/blackbox/common/BlackboxTestsTransportUDP.cpp
Commit date: 2024-04-02 11:00:21 +0200
Commit message: Merge pull request from GHSA-53xw-465j-rxfh

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
Diff: diff --git a/include/fastdds/rtps/messages/CDRMessage.h b/include/fastdds/rtps/messages/CDRMessage.h
index 738c65b96b..cb6e73b3db 100644
--- a/include/fastdds/rtps/messages/CDRMessage.h
+++ b/include/fastdds/rtps/messages/CDRMessage.h
@@ -138,19 +138,22 @@ inline bool readBinaryProperty(
         CDRMessage_t* msg,
         BinaryProperty& binary_property);
 
 inline bool readPropertySeq(
         CDRMessage_t* msg,
-        PropertySeq& properties);
+        PropertySeq& properties,
+        const uint32_t parameter_length);
 
 inline bool readBinaryPropertySeq(
         CDRMessage_t* msg,
-        BinaryPropertySeq& binary_properties);
+        BinaryPropertySeq& binary_properties,
+        const uint32_t parameter_length);
 
 inline bool readDataHolder(
         CDRMessage_t* msg,
-        DataHolder& data_holder);
+        DataHolder& data_holder,
+        const uint32_t parameter_length);
 
 inline bool readDataHolderSeq(
         CDRMessage_t* msg,
         DataHolderSeq& data_holders);
 
diff --git a/include/fastdds/rtps/messages/CDRMessage.hpp b/include/fastdds/rtps/messages/CDRMessage.hpp
index d74f28bf8d..4618dc361c 100644
--- a/include/fastdds/rtps/messages/CDRMessage.hpp
+++ b/include/fastdds/rtps/messages/CDRMessage.hpp
@@ -858,20 +858,28 @@ inline bool CDRMessage::addPropertySeq(
     return returnedValue;
 }
 
 inline bool CDRMessage::readPropertySeq(
         CDRMessage_t* msg,
-        PropertySeq& properties)
+        PropertySeq& properties,
+        const uint32_t parameter_length)
 {
     assert(msg);
 
     uint32_t length = 0;
     if (!CDRMessage::readUInt32(msg, &length))
     {
         return false;
     }
 
+    // Length should be at least 16 times the number of elements, since each property contains
+    // 2 empty strings, each with 4 bytes for its length + at least 4 bytes of data (single NUL character + padding)
+    if (16 * length > parameter_length)
+    {
+        return false;
+    }
+
     properties.resize(length);
     bool returnedValue = true;
     for (uint32_t i = 0; returnedValue && i < length; ++i)
     {
         returnedValue = CDRMessage::readProperty(msg, properties.at(i));
@@ -960,20 +968,29 @@ inline bool CDRMessage::addBinaryPropertySeq(
     return returnedValue;
 }
 
 inline bool CDRMessage::readBinaryPropertySeq(
         CDRMessage_t* msg,
-        BinaryPropertySeq& binary_properties)
+        BinaryPropertySeq& binary_properties,
+        const uint32_t parameter_length)
 {
     assert(msg);
 
     uint32_t length = 0;
     if (!CDRMessage::readUInt32(msg, &length))
     {
         return false;
     }
 
+    // Length should be at least 12 times the number of elements, since each each property contains at least
+    // 1 empty string with 4 bytes for its length + at least 4 bytes of data (single NUL character + padding) and
+    // 1 empty byte sequence with 4 bytes for its length
+    if (12 * length > parameter_length)
+    {
+        return false;
+    }
+
     binary_properties.resize(length);
     bool returnedValue = true;
     for (uint32_t i = 0; returnedValue && i < length; ++i)
     {
         returnedValue = CDRMessage::readBinaryProperty(msg, binary_properties.at(i));
@@ -1004,23 +1021,24 @@ inline bool CDRMessage::addDataHolder(
     return true;
 }
 
 inline bool CDRMessage::readDataHolder(
         CDRMessage_t* msg,
-        DataHolder& data_holder)
+        DataHolder& data_holder,
+        const uint32_t parameter_length)
 {
     assert(msg);
 
     if (!CDRMessage::readString(msg, &data_holder.class_id()))
     {
         return false;
     }
-    if (!CDRMessage::readPropertySeq(msg, data_holder.properties()))
+    if (!CDRMessage::readPropertySeq(msg, data_holder.properties(), parameter_length))
     {
         return false;
     }
-    if (!CDRMessage::readBinaryPropertySeq(msg, data_holder.binary_properties()))
+    if (!CDRMessage::readBinaryPropertySeq(msg, data_holder.binary_properties(), parameter_length))
     {
         return false;
     }
 
     return true;
@@ -1061,15 +1079,25 @@ inline bool CDRMessage::readDataHolderSeq(
     if (!CDRMessage::readUInt32(msg, &length))
     {
         return false;
     }
 
+    // Length should be at least 16 times the number of elements, since each DataHolder contains at least
+    // 1 empty string with 4 bytes for its length + at least 4 bytes of data (single NUL character + padding) and
+    // 2 empty property sequences, each with 4 bytes for its length
+    if (msg->pos + 16 * length > msg->length)
+    {
+        return false;
+    }
+
     data_holders.resize(length);
     bool returnedValue = true;
     for (uint32_t i = 0; returnedValue && i < length; ++i)
     {
-        returnedValue = CDRMessage::readDataHolder(msg, data_holders.at(i));
+        //! The parameter length should be the remaining length of the message
+        uint32_t remaining_length = msg->length - msg->pos;
+        returnedValue = CDRMessage::readDataHolder(msg, data_holders.at(i), remaining_length);
     }
 
     return returnedValue;
 }
 
diff --git a/src/cpp/fastdds/core/policy/ParameterSerializer.hpp b/src/cpp/fastdds/core/policy/ParameterSerializer.hpp
index d53a25edc5..a63c1af43a 100644
--- a/src/cpp/fastdds/core/policy/ParameterSerializer.hpp
+++ b/src/cpp/fastdds/core/policy/ParameterSerializer.hpp
@@ -1037,11 +1037,11 @@ inline bool ParameterSerializer<ParameterToken_t>::read_content_from_cdr_message
         const uint16_t parameter_length)
 {
 
     parameter.length = parameter_length;
     uint32_t pos_ref = cdr_message->pos;
-    bool valid =  fastrtps::rtps::CDRMessage::readDataHolder(cdr_message, parameter.token);
+    bool valid =  fastrtps::rtps::CDRMessage::readDataHolder(cdr_message, parameter.token, parameter_length);
     uint32_t length_diff = cdr_message->pos - pos_ref;
     valid &= (parameter_length == length_diff);
     return valid;
 }
 
diff --git a/test/blackbox/common/BlackboxTestsTransportUDP.cpp b/test/blackbox/common/BlackboxTestsTransportUDP.cpp
index e907d1afab..fd9c8e6635 100644
--- a/test/blackbox/common/BlackboxTestsTransportUDP.cpp
+++ b/test/blackbox/common/BlackboxTestsTransportUDP.cpp
@@ -531,10 +531,12 @@ TEST(TransportUDP, DatagramInjection)
     auto receivers = transport->get_receivers();
     ASSERT_FALSE(receivers.empty());
 
     DatagramInjectionTransport::deliver_datagram_from_file(receivers, "datagrams/16784.bin");
     DatagramInjectionTransport::deliver_datagram_from_file(receivers, "datagrams/20140.bin");
+    DatagramInjectionTransport::deliver_datagram_from_file(receivers, "datagrams/20574.bin");
+    DatagramInjectionTransport::deliver_datagram_from_file(receivers, "datagrams/20660.bin");
 }
 
 TEST(TransportUDP, MaliciousManipulatedDataOctetsToNextHeaderIgnore)
 {
     // Force using UDP transport
diff --git a/test/blackbox/datagrams/20574.bin b/test/blackbox/datagrams/20574.bin
new file mode 100644
index 0000000000..1aba115252
Binary files /dev/null and b/test/blackbox/datagrams/20574.bin differ
diff --git a/test/blackbox/datagrams/20660.bin b/test/blackbox/datagrams/20660.bin
new file mode 100644
index 0000000000..ff9122757f
Binary files /dev/null and b/test/blackbox/datagrams/20660.bin differ

Vulnerable code:         PropertySeq& properties);
        BinaryPropertySeq& binary_properties);
        DataHolder& data_holder);
        PropertySeq& properties)
        BinaryPropertySeq& binary_properties)
        DataHolder& data_holder)
    if (!CDRMessage::readPropertySeq(msg, data_holder.properties()))
    if (!CDRMessage::readBinaryPropertySeq(msg, data_holder.binary_properties()))
        returnedValue = CDRMessage::readDataHolder(msg, data_holders.at(i));
    bool valid =  fastrtps::rtps::CDRMessage::readDataHolder(cdr_message, parameter.token);
Fixed code:         PropertySeq& properties,
        const uint32_t parameter_length);
        BinaryPropertySeq& binary_properties,
        const uint32_t parameter_length);
        DataHolder& data_holder,
        const uint32_t parameter_length);
        PropertySeq& properties,
        const uint32_t parameter_length)
    // Length should be at least 16 times the number of elements, since each property contains
    // 2 empty strings, each with 4 bytes for its length + at least 4 bytes of data (single NUL character + padding)
    if (16 * length > parameter_length)
    {
        return false;
    }

        BinaryPropertySeq& binary_properties,
        const uint32_t parameter_length)
    // Length should be at least 12 times the number of elements, since each each property contains at least
    // 1 empty string with 4 bytes for its length + at least 4 bytes of data (single NUL character + padding) and
    // 1 empty byte sequence with 4 bytes for its length
    if (12 * length > parameter_length)
    {
        return false;
    }

        DataHolder& data_holder,
        const uint32_t parameter_length)
    if (!CDRMessage::readPropertySeq(msg, data_holder.properties(), parameter_length))
    if (!CDRMessage::readBinaryPropertySeq(msg, data_holder.binary_properties(), parameter_length))
    // Length should be at least 16 times the number of elements, since each DataHolder contains at least
    // 1 empty string with 4 bytes for its length + at least 4 bytes of data (single NUL character + padding) and
    // 2 empty property sequences, each with 4 bytes for its length
    if (msg->pos + 16 * length > msg->length)
    {
        return false;
    }

        //! The parameter length should be the remaining length of the message
        uint32_t remaining_length = msg->length - msg->pos;
        returnedValue = CDRMessage::readDataHolder(msg, data_holders.at(i), remaining_length);
    bool valid =  fastrtps::rtps::CDRMessage::readDataHolder(cdr_message, parameter.token, parameter_length);
    DatagramInjectionTransport::deliver_datagram_from_file(receivers, "datagrams/20574.bin");
    DatagramInjectionTransport::deliver_datagram_from_file(receivers, "datagrams/20660.bin");