From edc29883e9644f62aa95686ebdf9e8f2c150a7c1 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 3 Sep 2025 07:41:04 +0900 Subject: [PATCH] Test failing deserialization of invalid sequence length (backport #261) (#264) * Test failing deserialization of invalid sequence length (#261) * Add test infrastructure. Signed-off-by: Miguel Company * Test that deserialization with wrong sequence length fails. Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company (cherry picked from commit 4dd5d571a5bfa1a67183acf271dfa442932c7572) # Conflicts: # test_rmw_implementation/test/test_serialize_deserialize.cpp * resolve conflicts. Signed-off-by: Tomoya Fujita --------- Signed-off-by: Tomoya Fujita Co-authored-by: Miguel Company Co-authored-by: Tomoya Fujita --- .../test/test_serialize_deserialize.cpp | 122 +++++++++++++++++- 1 file changed, 119 insertions(+), 3 deletions(-) diff --git a/test_rmw_implementation/test/test_serialize_deserialize.cpp b/test_rmw_implementation/test/test_serialize_deserialize.cpp index 5302229..3ed1568 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; + } +} + #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX # define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX) @@ -44,7 +124,7 @@ class CLASSNAME (TestSerializeDeserialize, RMW_IMPLEMENTATION) : public ::testin { }; -TEST_F(CLASSNAME(TestSerializeDeserialize, RMW_IMPLEMENTATION), get_serialization_format) { +TEST(TestSerializeDeserialize, get_serialization_format) { const char * serialization_format = rmw_get_serialization_format(); EXPECT_NE(nullptr, serialization_format); EXPECT_STREQ(serialization_format, rmw_get_serialization_format()); @@ -185,7 +265,27 @@ TEST_F( EXPECT_EQ(input_message.uint16_values.data[0], output_message.uint16_values.data[0]); } -TEST_F(CLASSNAME(TestSerializeDeserialize, RMW_IMPLEMENTATION), clean_round_trip_for_cpp_message) { +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(); test_msgs::msg::BasicTypes input_message{}; @@ -261,7 +361,23 @@ TEST_F( EXPECT_EQ(input_message, output_message); } -TEST_F(CLASSNAME(TestSerializeDeserialize, RMW_IMPLEMENTATION), rmw_get_serialized_message_size) +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) { // TODO(anyone): Add tests here when the implementation it's supported