From a7db732d7f005a0520fb3ac91cd22c501a5efa55 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 17 Jul 2025 21:07:49 -0700 Subject: [PATCH 1/3] Test failing deserialization of invalid sequence length (#261) (#263) * Add test infrastructure. * Test that deserialization with wrong sequence length fails. --------- (cherry picked from commit 4dd5d571a5bfa1a67183acf271dfa442932c7572) Signed-off-by: Miguel Company Co-authored-by: Miguel Company --- .../test/test_serialize_deserialize.cpp | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/test_rmw_implementation/test/test_serialize_deserialize.cpp b/test_rmw_implementation/test/test_serialize_deserialize.cpp index e40e15a..2b32c9f 100644 --- a/test_rmw_implementation/test/test_serialize_deserialize.cpp +++ b/test_rmw_implementation/test/test_serialize_deserialize.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include +#include #include "osrf_testing_tools_cpp/scope_exit.hpp" @@ -31,8 +32,87 @@ #include "test_msgs/msg/bounded_plain_sequences.h" #include "test_msgs/msg/bounded_plain_sequences.hpp" +#include "test_msgs/msg/unbounded_sequences.h" +#include "test_msgs/msg/unbounded_sequences.hpp" + #include "./allocator_testing_utils.h" +static void check_bad_cdr_sequence_cases( + const rosidl_message_type_support_t * ts, + void * message) +{ + // Serialized CDR buffer for a data with all sequences empty. + constexpr size_t kBufferSize = 132; + const uint8_t valid_data[kBufferSize] = { + 0x01, 0x00, 0x00, 0x00, // representation header (CDR little endian) + 0x00, 0x00, 0x00, 0x00, // bool[] bool_values + 0x00, 0x00, 0x00, 0x00, // byte[] byte_values + 0x00, 0x00, 0x00, 0x00, // char[] char_values + 0x00, 0x00, 0x00, 0x00, // float32[] float32_values + 0x00, 0x00, 0x00, 0x00, // float64[] float64_values + 0x00, 0x00, 0x00, 0x00, // int8[] int8_values + 0x00, 0x00, 0x00, 0x00, // uint8[] uint8_values + 0x00, 0x00, 0x00, 0x00, // int16[] int16_values + 0x00, 0x00, 0x00, 0x00, // uint16[] uint16_values + 0x00, 0x00, 0x00, 0x00, // int32[] int32_values + 0x00, 0x00, 0x00, 0x00, // uint32[] uint32_values + 0x00, 0x00, 0x00, 0x00, // int64[] int64_values + 0x00, 0x00, 0x00, 0x00, // uint64[] uint64_values + 0x00, 0x00, 0x00, 0x00, // string[] string_values + 0x00, 0x00, 0x00, 0x00, // BasicTypes[] basic_types_values + 0x00, 0x00, 0x00, 0x00, // Constants[] constants_values + 0x00, 0x00, 0x00, 0x00, // Defaults[] defaults_values + 0x00, 0x00, 0x00, 0x00, // bool[] bool_values_default + 0x00, 0x00, 0x00, 0x00, // byte[] byte_values_default + 0x00, 0x00, 0x00, 0x00, // char[] char_values_default + 0x00, 0x00, 0x00, 0x00, // float32[] float32_values_default + 0x00, 0x00, 0x00, 0x00, // float64[] float64_values_default + 0x00, 0x00, 0x00, 0x00, // int8[] int8_values_default + 0x00, 0x00, 0x00, 0x00, // uint8[] uint8_values_default + 0x00, 0x00, 0x00, 0x00, // int16[] int16_values_default + 0x00, 0x00, 0x00, 0x00, // uint16[] uint16_values_default + 0x00, 0x00, 0x00, 0x00, // int32[] int32_values_default + 0x00, 0x00, 0x00, 0x00, // uint32[] uint32_values_default + 0x00, 0x00, 0x00, 0x00, // int64[] int64_values_default + 0x00, 0x00, 0x00, 0x00, // uint64[] uint64_values_default + 0x00, 0x00, 0x00, 0x00, // string[] string_values_default + 0x00, 0x00, 0x00, 0x00 // int32 alignment_check + }; + + uint8_t buffer[kBufferSize]; + memcpy(buffer, valid_data, kBufferSize); + + // The first 4 bytes are the CDR representation header, which we don't modify. + constexpr size_t kFirstSequenceOffset = 4; + // The last 4 bytes are the alignment check, which we also don't modify. + constexpr size_t kLastSequenceOffset = kBufferSize - 4; + // Each sequence length is stored as a 4-byte unsigned integer. + constexpr size_t kSequenceLengthSize = 4; + + for (size_t i = kFirstSequenceOffset; i < kLastSequenceOffset; i += kSequenceLengthSize) { + // Corrupt the buffer by changing the size of a sequence to an invalid value. + buffer[i] = 0xFF; + buffer[i + 1] = 0xFF; + buffer[i + 2] = 0xFF; + buffer[i + 3] = 0xFF; + + // Expect the deserialization to fail. + rmw_serialized_message_t serialized_message = rmw_get_zero_initialized_serialized_message(); + serialized_message.buffer = const_cast(buffer); + serialized_message.buffer_length = sizeof(buffer); + serialized_message.buffer_capacity = sizeof(buffer); + rmw_ret_t ret = rmw_deserialize(&serialized_message, ts, message); + EXPECT_NE(RMW_RET_OK, ret); + rmw_reset_error(); + + // Restore the buffer to a valid state. + buffer[i] = 0x00; + buffer[i + 1] = 0x00; + buffer[i + 2] = 0x00; + buffer[i + 3] = 0x00; + } +} + TEST(TestSerializeDeserialize, get_serialization_format) { const char * serialization_format = rmw_get_serialization_format(); EXPECT_NE(nullptr, serialization_format); @@ -171,6 +251,26 @@ TEST(TestSerializeDeserialize, clean_round_trip_for_c_bounded_message) { EXPECT_EQ(input_message.uint16_values.data[0], output_message.uint16_values.data[0]); } +TEST(TestSerializeDeserialize, bad_cdr_sequence_correctly_fails_for_c) { + { + const char * serialization_format = rmw_get_serialization_format(); + if (0 != strcmp(serialization_format, "cdr")) { + GTEST_SKIP(); + } + } + + const rosidl_message_type_support_t * ts{ + ROSIDL_GET_MSG_TYPE_SUPPORT(test_msgs, msg, UnboundedSequences)}; + test_msgs__msg__UnboundedSequences output_message{}; + ASSERT_TRUE(test_msgs__msg__UnboundedSequences__init(&output_message)); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + test_msgs__msg__UnboundedSequences__fini(&output_message); + }); + + check_bad_cdr_sequence_cases(ts, &output_message); +} + TEST(TestSerializeDeserialize, clean_round_trip_for_cpp_message) { const rosidl_message_type_support_t * ts = rosidl_typesupport_cpp::get_message_type_support_handle(); @@ -244,6 +344,22 @@ TEST(TestSerializeDeserialize, clean_round_trip_for_cpp_bounded_message) { EXPECT_EQ(input_message, output_message); } +TEST(TestSerializeDeserialize, bad_cdr_sequence_correctly_fails_for_cpp) { + { + const char * serialization_format = rmw_get_serialization_format(); + if (0 != strcmp(serialization_format, "cdr")) { + GTEST_SKIP(); + } + } + + using TestMessage = test_msgs::msg::UnboundedSequences; + const rosidl_message_type_support_t * ts = + rosidl_typesupport_cpp::get_message_type_support_handle(); + TestMessage output_message{}; + + check_bad_cdr_sequence_cases(ts, &output_message); +} + TEST(TestSerializeDeserialize, rmw_get_serialized_message_size) { if (rmw_get_serialized_message_size(nullptr, nullptr, nullptr) != RMW_RET_UNSUPPORTED) { From 01241deaf4d253a33f3cd90e2055a12a27e1a00d Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Tue, 5 Aug 2025 20:23:29 +0800 Subject: [PATCH 2/3] Changelog. Signed-off-by: Marco A. Gutierrez --- rmw_implementation/CHANGELOG.rst | 7 +++++++ test_rmw_implementation/CHANGELOG.rst | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/rmw_implementation/CHANGELOG.rst b/rmw_implementation/CHANGELOG.rst index e89613a..f36af9e 100644 --- a/rmw_implementation/CHANGELOG.rst +++ b/rmw_implementation/CHANGELOG.rst @@ -2,6 +2,13 @@ Changelog for package rmw_implementation ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Fixed windows warning (`#254 `_) (`#260 `_) + (cherry picked from commit 8c006087e3af66a79f61dbe0ff45e7a1ef31a686) + Co-authored-by: Alejandro Hernández Cordero +* Contributors: mergify[bot] + 2.15.5 (2025-03-12) ------------------- * Added rmw_event_type_is_supported (`#250 `_) (`#252 `_) diff --git a/test_rmw_implementation/CHANGELOG.rst b/test_rmw_implementation/CHANGELOG.rst index 65d53b9..222f18b 100644 --- a/test_rmw_implementation/CHANGELOG.rst +++ b/test_rmw_implementation/CHANGELOG.rst @@ -2,6 +2,19 @@ Changelog for package test_rmw_implementation ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Test failing deserialization of invalid sequence length (`#261 `_) (`#263 `_) + * Add test infrastructure. + * Test that deserialization with wrong sequence length fails. + --------- + (cherry picked from commit 4dd5d571a5bfa1a67183acf271dfa442932c7572) + Co-authored-by: Miguel Company +* add ignore_local_publications_serialized test. (`#255 `_) (`#257 `_) + (cherry picked from commit 1eceed45fbdbe6b93bb49993f1bed9698aeca38f) + Co-authored-by: Tomoya Fujita +* Contributors: mergify[bot] + 2.15.5 (2025-03-12) ------------------- * Added rmw_event_type_is_supported (`#250 `_) (`#252 `_) From 23fe893a860545da06ec98d0a22fdc8554d5c0ac Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Tue, 5 Aug 2025 20:24:05 +0800 Subject: [PATCH 3/3] 2.15.6 Signed-off-by: Marco A. Gutierrez --- rmw_implementation/CHANGELOG.rst | 4 ++-- rmw_implementation/package.xml | 2 +- test_rmw_implementation/CHANGELOG.rst | 4 ++-- test_rmw_implementation/package.xml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rmw_implementation/CHANGELOG.rst b/rmw_implementation/CHANGELOG.rst index f36af9e..c388bc7 100644 --- a/rmw_implementation/CHANGELOG.rst +++ b/rmw_implementation/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package rmw_implementation ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +2.15.6 (2025-08-05) +------------------- * Fixed windows warning (`#254 `_) (`#260 `_) (cherry picked from commit 8c006087e3af66a79f61dbe0ff45e7a1ef31a686) Co-authored-by: Alejandro Hernández Cordero diff --git a/rmw_implementation/package.xml b/rmw_implementation/package.xml index f88e828..7b805e9 100644 --- a/rmw_implementation/package.xml +++ b/rmw_implementation/package.xml @@ -2,7 +2,7 @@ rmw_implementation - 2.15.5 + 2.15.6 Proxy implementation of the ROS 2 Middleware Interface. William Woodall diff --git a/test_rmw_implementation/CHANGELOG.rst b/test_rmw_implementation/CHANGELOG.rst index 222f18b..07e0684 100644 --- a/test_rmw_implementation/CHANGELOG.rst +++ b/test_rmw_implementation/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package test_rmw_implementation ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +2.15.6 (2025-08-05) +------------------- * Test failing deserialization of invalid sequence length (`#261 `_) (`#263 `_) * Add test infrastructure. * Test that deserialization with wrong sequence length fails. diff --git a/test_rmw_implementation/package.xml b/test_rmw_implementation/package.xml index cbb544c..4781f9c 100644 --- a/test_rmw_implementation/package.xml +++ b/test_rmw_implementation/package.xml @@ -2,7 +2,7 @@ test_rmw_implementation - 2.15.5 + 2.15.6 Test suite for ROS middleware API. William Woodall