From e197fffebca3bef8a062cd33cefd10b7e81b5527 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 20 Sep 2023 21:31:56 +0200 Subject: [PATCH] Fix bad-free when receiving malformed DATA submessage (#3861) * Refs #16784. Added basic custom chaining transport. Signed-off-by: Miguel Company * Refs #16784. Use custom transport to get list of receiver interfaces. Signed-off-by: Miguel Company * Refs #16784. Added regression file. Signed-off-by: Miguel Company * Refs #16784. Processing regression files. Signed-off-by: Miguel Company * Refs #16784. Separation of transport and descriptor. Signed-off-by: Miguel Company * Refs #16784. Small refactor. Signed-off-by: Miguel Company * Refs #16784. Fix issue. Signed-off-by: Miguel Company * Refs #19416. Fix build error in Mac. Signed-off-by: Miguel Company * Refs #19416. Fix include order. Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company (cherry picked from commit 47fe5d7636769706fcf71bf7d183c34c64837e37) Co-authored-by: Miguel Company --- src/cpp/rtps/messages/MessageReceiver.cpp | 4 + test/blackbox/CMakeLists.txt | 2 + .../common/BlackboxTestsTransportUDP.cpp | 53 ++++++++- .../common/DatagramInjectionTransport.cpp | 47 ++++++++ .../common/DatagramInjectionTransport.hpp | 102 ++++++++++++++++++ test/blackbox/datagrams/16784.bin | Bin 0 -> 48 bytes 6 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 test/blackbox/common/DatagramInjectionTransport.cpp create mode 100644 test/blackbox/common/DatagramInjectionTransport.hpp create mode 100644 test/blackbox/datagrams/16784.bin diff --git a/src/cpp/rtps/messages/MessageReceiver.cpp b/src/cpp/rtps/messages/MessageReceiver.cpp index 9949e1df084..8d3ad8e1c29 100644 --- 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 index df7ed125157..540be791123 100644 --- 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 index 0cccf3e96d8..1ce54611af4 100644 --- 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 +#include +#include +#include +#include #include #include #include -#include #include +#include + +#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& receivers, + const char* filename) +{ + std::basic_ifstream 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 buf(file_size); + file.read(reinterpret_cast(buf.data()), file_size); + + eprosima::fastdds::rtps::Locator loc; + for (const auto& rec : receivers) + { + rec->OnDataReceived(buf.data(), static_cast(file_size), loc, loc); + } +} + +TEST(TransportUDP, DatagramInjection) +{ + using eprosima::fastdds::rtps::DatagramInjectionTransportDescriptor; + + auto low_level_transport = std::make_shared(); + auto transport = std::make_shared(low_level_transport); + + PubSubWriter 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 new file mode 100644 index 00000000000..a4b74f43b3f --- /dev/null +++ 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 low_level) + : ChainingTransportDescriptor(low_level) +{ +} + +TransportInterface* DatagramInjectionTransportDescriptor::create_transport() const +{ + return new DatagramInjectionTransport(const_cast(this)); +} + +void DatagramInjectionTransportDescriptor::add_receiver( + TransportReceiverInterface* receiver_interface) +{ + std::lock_guard guard(mtx_); + receivers_.insert(receiver_interface); +} + +std::set DatagramInjectionTransportDescriptor::get_receivers() +{ + std::lock_guard guard(mtx_); + return receivers_; +} + +} // namespace rtps +} // namespace fastdds +} // namespace eprosima diff --git a/test/blackbox/common/DatagramInjectionTransport.hpp b/test/blackbox/common/DatagramInjectionTransport.hpp new file mode 100644 index 00000000000..3cfff45d6c9 --- /dev/null +++ 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 +#include + +#include +#include + +namespace eprosima { +namespace fastdds { +namespace rtps { + +class DatagramInjectionTransportDescriptor : public ChainingTransportDescriptor +{ + +public: + + DatagramInjectionTransportDescriptor( + std::shared_ptr low_level); + + TransportInterface* create_transport() const override; + + void add_receiver( + TransportReceiverInterface* receiver_interface); + + std::set get_receivers(); + +private: + + std::mutex mtx_; + std::set 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 new file mode 100644 index 0000000000000000000000000000000000000000..678858b8c47d8e6201f63083a82ba479d3c7240e GIT binary patch literal 48 wcmWFv2?%ClVdQ6IW^!61$iU?K^wD2YaR~+n27yUHf`NgN;Sh*o0y7yH0N%g{8~^|S literal 0 HcmV?d00001