Fix bad-free when receiving malformed DATA submessage (#3861)

* Refs #16784. Added basic custom chaining transport. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs #16784. Use custom transport to get list of receiver interfaces. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs #16784. Added regression file. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs #16784. Processing regression files. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs #16784. Separation of transport and descriptor. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs #16784. Small refactor. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs #16784. Fix issue. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs #19416. Fix build error in Mac. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> * Refs #19416. Fix include order. Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> --------- Signed-off-by: Miguel Company <MiguelCompany@eprosima.com> (cherry picked from commit 47fe5d7) Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
eProsima · Sep 20, 2023 · e197fff · e197fff
1 parent 208ca45
commit e197fff
Show file tree

Hide file tree

Showing 6 changed files with 203 additions and 5 deletions.
diff --git a/src/cpp/rtps/messages/MessageReceiver.cpp b/src/cpp/rtps/messages/MessageReceiver.cpp
@@ -841,6 +841,8 @@ bool MessageReceiver::proc_Submsg_Data(
             {
                 EPROSIMA_LOG_WARNING(RTPS_MSG_IN, IDSTRING "Serialized Payload value invalid or larger than maximum allowed size"
                         "(" << payload_size << "/" << (msg->length - msg->pos) << ")");
+                ch.serializedPayload.data = nullptr;
+                ch.inline_qos.data = nullptr;
                 return false;
             }
         }
@@ -849,6 +851,8 @@ bool MessageReceiver::proc_Submsg_Data(
             if (payload_size <= 0)
             {
                 EPROSIMA_LOG_WARNING(RTPS_MSG_IN, IDSTRING "Serialized Payload value invalid (" << payload_size << ")");
+                ch.serializedPayload.data = nullptr;
+                ch.inline_qos.data = nullptr;
                 return false;
             }
 

diff --git a/test/blackbox/CMakeLists.txt b/test/blackbox/CMakeLists.txt
@@ -302,6 +302,7 @@ set(BLACKBOXTESTS_SOURCE ${BLACKBOXTESTS_TEST_SOURCE}
     utils/lambda_functions.cpp
     utils/print_functions.cpp
 
+    common/DatagramInjectionTransport.cpp
     common/TCPReqRepHelloWorldRequester.cpp
     common/TCPReqRepHelloWorldReplier.cpp
     )
@@ -340,6 +341,7 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/utils/check_guid.py
     ${CMAKE_CURRENT_BINARY_DIR}/check_guid.py)
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/partitions_profile.xml
     ${CMAKE_CURRENT_BINARY_DIR}/partitions_profile.xml)
+file(COPY "${CMAKE_CURRENT_SOURCE_DIR}/datagrams" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}")
 
 if(FASTRTPS_API_TESTS)
     set(BLACKBOXTESTS_FASTRTPS_SOURCE

diff --git a/test/blackbox/common/BlackboxTestsTransportUDP.cpp b/test/blackbox/common/BlackboxTestsTransportUDP.cpp
@@ -12,17 +12,23 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "BlackboxTests.hpp"
-
-#include "PubSubReader.hpp"
-#include "PubSubWriter.hpp"
+#include <cstdint>
+#include <fstream>
+#include <mutex>
+#include <set>
+#include <vector>
 
 #include <gtest/gtest.h>
 
 #include <fastdds/rtps/transport/UDPv4TransportDescriptor.h>
 #include <fastdds/rtps/transport/UDPv6TransportDescriptor.h>
-#include <fastrtps/utils/IPFinder.h>
 #include <fastrtps/log/Log.h>
+#include <fastrtps/utils/IPFinder.h>
+
+#include "BlackboxTests.hpp"
+#include "DatagramInjectionTransport.hpp"
+#include "PubSubReader.hpp"
+#include "PubSubWriter.hpp"
 
 using namespace eprosima::fastrtps;
 using namespace eprosima::fastrtps::rtps;
@@ -507,6 +513,43 @@ TEST_P(TransportUDP, whitelisting_udp_localhost_alone)
     }
 }
 
+void deliver_datagram_from_file(
+        const std::set<eprosima::fastdds::rtps::TransportReceiverInterface*>& receivers,
+        const char* filename)
+{
+    std::basic_ifstream<char> file(filename, std::ios::binary | std::ios::in);
+
+    file.seekg(0, file.end);
+    size_t file_size = file.tellg();
+    file.seekg(0, file.beg);
+
+    std::vector<uint8_t> buf(file_size);
+    file.read(reinterpret_cast<char*>(buf.data()), file_size);
+
+    eprosima::fastdds::rtps::Locator loc;
+    for (const auto& rec : receivers)
+    {
+        rec->OnDataReceived(buf.data(), static_cast<uint32_t>(file_size), loc, loc);
+    }
+}
+
+TEST(TransportUDP, DatagramInjection)
+{
+    using eprosima::fastdds::rtps::DatagramInjectionTransportDescriptor;
+
+    auto low_level_transport = std::make_shared<UDPv4TransportDescriptor>();
+    auto transport = std::make_shared<DatagramInjectionTransportDescriptor>(low_level_transport);
+
+    PubSubWriter<HelloWorldPubSubType> writer(TEST_TOPIC_NAME);
+    writer.disable_builtin_transport().add_user_transport_to_pparams(transport).init();
+    ASSERT_TRUE(writer.isInitialized());
+
+    auto receivers = transport->get_receivers();
+    ASSERT_FALSE(receivers.empty());
+
+    deliver_datagram_from_file(receivers, "datagrams/16784.bin");
+}
+
 // Test for ==operator UDPTransportDescriptor is not required as it is an abstract class and in UDPv4 is same method
 // Test for copy UDPTransportDescriptor is not required as it is an abstract class and in UDPv4 is same method
 

diff --git a/test/blackbox/common/DatagramInjectionTransport.cpp b/test/blackbox/common/DatagramInjectionTransport.cpp
@@ -0,0 +1,47 @@
+// Copyright 2023 Proyectos y Sistemas de Mantenimiento SL (eProsima).
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "./DatagramInjectionTransport.hpp"
+
+namespace eprosima {
+namespace fastdds {
+namespace rtps {
+
+DatagramInjectionTransportDescriptor::DatagramInjectionTransportDescriptor(
+        std::shared_ptr<TransportDescriptorInterface> low_level)
+    : ChainingTransportDescriptor(low_level)
+{
+}
+
+TransportInterface* DatagramInjectionTransportDescriptor::create_transport() const
+{
+    return new DatagramInjectionTransport(const_cast<DatagramInjectionTransportDescriptor*>(this));
+}
+
+void DatagramInjectionTransportDescriptor::add_receiver(
+        TransportReceiverInterface* receiver_interface)
+{
+    std::lock_guard<std::mutex> guard(mtx_);
+    receivers_.insert(receiver_interface);
+}
+
+std::set<TransportReceiverInterface*> DatagramInjectionTransportDescriptor::get_receivers()
+{
+    std::lock_guard<std::mutex> guard(mtx_);
+    return receivers_;
+}
+
+} // namespace rtps
+} // namespace fastdds
+} // namespace eprosima
diff --git a/test/blackbox/common/DatagramInjectionTransport.hpp b/test/blackbox/common/DatagramInjectionTransport.hpp
@@ -0,0 +1,102 @@
+// Copyright 2023 Proyectos y Sistemas de Mantenimiento SL (eProsima).
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <mutex>
+#include <set>
+
+#include <fastdds/rtps/transport/ChainingTransport.h>
+#include <fastdds/rtps/transport/ChainingTransportDescriptor.h>
+
+namespace eprosima {
+namespace fastdds {
+namespace rtps {
+
+class DatagramInjectionTransportDescriptor : public ChainingTransportDescriptor
+{
+
+public:
+
+    DatagramInjectionTransportDescriptor(
+            std::shared_ptr<TransportDescriptorInterface> low_level);
+
+    TransportInterface* create_transport() const override;
+
+    void add_receiver(
+            TransportReceiverInterface* receiver_interface);
+
+    std::set<TransportReceiverInterface*> get_receivers();
+
+private:
+
+    std::mutex mtx_;
+    std::set<TransportReceiverInterface*> receivers_;
+};
+
+class DatagramInjectionTransport : public ChainingTransport
+{
+public:
+
+    DatagramInjectionTransport(
+            DatagramInjectionTransportDescriptor* parent)
+        : ChainingTransport(*parent)
+        , parent_(parent)
+    {
+    }
+
+    TransportDescriptorInterface* get_configuration() override
+    {
+        return parent_;
+    }
+
+    bool send(
+            eprosima::fastrtps::rtps::SenderResource* /*low_sender_resource*/,
+            const eprosima::fastrtps::rtps::octet* /*send_buffer*/,
+            uint32_t /*send_buffer_size*/,
+            eprosima::fastrtps::rtps::LocatorsIterator* /*destination_locators_begin*/,
+            eprosima::fastrtps::rtps::LocatorsIterator* /*destination_locators_end*/,
+            const std::chrono::steady_clock::time_point& /*timeout*/) override
+    {
+        return true;
+    }
+
+    void receive(
+            TransportReceiverInterface* /*next_receiver*/,
+            const eprosima::fastrtps::rtps::octet* /*receive_buffer*/,
+            uint32_t /*receive_buffer_size*/,
+            const eprosima::fastrtps::rtps::Locator_t& /*local_locator*/,
+            const eprosima::fastrtps::rtps::Locator_t& /*remote_locator*/) override
+    {
+    }
+
+    bool OpenInputChannel(
+            const eprosima::fastrtps::rtps::Locator_t& loc,
+            TransportReceiverInterface* receiver_interface,
+            uint32_t max_message_size) override
+    {
+        bool ret_val = ChainingTransport::OpenInputChannel(loc, receiver_interface, max_message_size);
+        if (ret_val)
+        {
+            parent_->add_receiver(receiver_interface);
+        }
+        return ret_val;
+    }
+
+private:
+
+    DatagramInjectionTransportDescriptor* parent_ = nullptr;
+};
+
+} // namespace rtps
+} // namespace fastdds
+} // namespace eprosima
diff --git a/test/blackbox/datagrams/16784.bin b/test/blackbox/datagrams/16784.bin