From 5cda5b65ea56b6f126da8b34164ab6819f2b687b Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Sun, 3 May 2026 14:52:03 -0400 Subject: [PATCH] Fix UB in accessing the keys (#879) * Make sure to do key.resize() Doing a key.reserve() allocates memory, but still leaves the size at 0. Thus an access with the [] operator is UB. This came up on Ubuntu 26.04 because it is more strict about UB. Fix it by using resize() instead, which does increase the size and thus the [] operator is no longer UB. Signed-off-by: Chris Lalancette * rmw_fastrtps_dynamic_cpp: Fix a UB in key support. The problem here is that we were always accessing 16 bytes of data from the key buffer, even when it was shorter than that. What we do instead is to always set the ihandle->value to all zeros, then only copy in the number of key_buffer bytes we have. This matches what rmw_fastrtps_cpp does, and fixes a crash in tests on Ubuntu 26.04 Signed-off-by: Chris Lalancette --------- Signed-off-by: Chris Lalancette (cherry picked from commit eb157786ab3346e8d4291bbf8b5eeb6a34f41ced) --- rmw_fastrtps_cpp/src/type_support_common.cpp | 6 +++--- .../src/MessageTypeSupport_impl.hpp | 2 +- .../src/ServiceTypeSupport_impl.hpp | 4 ++-- .../src/TypeSupport_impl.hpp | 19 +++++++++++-------- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/rmw_fastrtps_cpp/src/type_support_common.cpp b/rmw_fastrtps_cpp/src/type_support_common.cpp index 6e92d52e5a..25d0d1b661 100644 --- a/rmw_fastrtps_cpp/src/type_support_common.cpp +++ b/rmw_fastrtps_cpp/src/type_support_common.cpp @@ -75,7 +75,7 @@ void TypeSupport::set_members(const message_type_support_callbacks_t * members) key_max_serialized_size_ = key_callbacks_->max_serialized_size_key(key_is_unbounded_); if (!key_is_unbounded_) { - key_buffer_.reserve(key_max_serialized_size_); + key_buffer_.resize(key_max_serialized_size_); } } } @@ -165,7 +165,7 @@ bool TypeSupport::get_key_hash_from_ros_message( key_max_serialized_size_ = (std::max) ( key_max_serialized_size_, key_callbacks_->get_serialized_size_key(ros_message)); - key_buffer_.reserve(key_max_serialized_size_); + key_buffer_.resize(key_max_serialized_size_); } eprosima::fastcdr::FastBuffer fast_buffer( @@ -179,7 +179,7 @@ bool TypeSupport::get_key_hash_from_ros_message( const size_t max_serialized_key_length = 16; - auto ser_length = ser.get_serialized_data_length(); + const auto ser_length = ser.get_serialized_data_length(); // check for md5 if (force_md5 || key_max_serialized_size_ > max_serialized_key_length) { diff --git a/rmw_fastrtps_dynamic_cpp/src/MessageTypeSupport_impl.hpp b/rmw_fastrtps_dynamic_cpp/src/MessageTypeSupport_impl.hpp index 84405be569..0be8a22241 100644 --- a/rmw_fastrtps_dynamic_cpp/src/MessageTypeSupport_impl.hpp +++ b/rmw_fastrtps_dynamic_cpp/src/MessageTypeSupport_impl.hpp @@ -65,7 +65,7 @@ MessageTypeSupport::MessageTypeSupport( if (this->members_->has_any_key_member_) { this->key_max_serialized_size_ = this->calculateMaxSerializedKeySize(members); this->is_compute_key_provided = true; - this->key_buffer_.reserve(this->key_max_serialized_size_); + this->key_buffer_.resize(this->key_max_serialized_size_); } // Account for RTPS submessage alignment diff --git a/rmw_fastrtps_dynamic_cpp/src/ServiceTypeSupport_impl.hpp b/rmw_fastrtps_dynamic_cpp/src/ServiceTypeSupport_impl.hpp index bed1661af5..7110430edd 100644 --- a/rmw_fastrtps_dynamic_cpp/src/ServiceTypeSupport_impl.hpp +++ b/rmw_fastrtps_dynamic_cpp/src/ServiceTypeSupport_impl.hpp @@ -66,7 +66,7 @@ RequestTypeSupport::RequestTypeSupport( if (this->members_->has_any_key_member_) { this->key_max_serialized_size_ = this->calculateMaxSerializedKeySize(this->members_); this->is_compute_key_provided = true; - this->key_buffer_.reserve(this->key_max_serialized_size_); + this->key_buffer_.resize(this->key_max_serialized_size_); } // Account for RTPS submessage alignment @@ -109,7 +109,7 @@ ResponseTypeSupport::ResponseTypeSupport if (this->members_->has_any_key_member_) { this->key_max_serialized_size_ = this->calculateMaxSerializedKeySize(this->members_); this->is_compute_key_provided = true; - this->key_buffer_.reserve(this->key_max_serialized_size_); + this->key_buffer_.resize(this->key_max_serialized_size_); } // Account for RTPS submessage alignment diff --git a/rmw_fastrtps_dynamic_cpp/src/TypeSupport_impl.hpp b/rmw_fastrtps_dynamic_cpp/src/TypeSupport_impl.hpp index 06c35c4bf4..68c33dd142 100644 --- a/rmw_fastrtps_dynamic_cpp/src/TypeSupport_impl.hpp +++ b/rmw_fastrtps_dynamic_cpp/src/TypeSupport_impl.hpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -363,7 +364,7 @@ bool TypeSupport::get_key_hash_from_ros_message( this->key_max_serialized_size_ = (std::max) (this->key_max_serialized_size_, this->getEstimatedSerializedKeySize(members, ros_message)); - key_buffer_.reserve(this->key_max_serialized_size_); + key_buffer_.resize(this->key_max_serialized_size_); } eprosima::fastcdr::FastBuffer buffer( @@ -376,22 +377,24 @@ bool TypeSupport::get_key_hash_from_ros_message( // serialize serializeKeyROSmessage(ser, members_, ros_message); - // check for md5 + const size_t max_serialized_key_length = 16; + + const auto ser_length = ser.get_serialized_data_length(); + if (force_md5 || this->key_max_serialized_size_ > 16) { md5_.init(); - md5_.update( - this->key_buffer_.data(), - static_cast(ser.get_serialized_data_length())); + md5_.update(key_buffer_.data(), static_cast(ser_length)); md5_.finalize(); - for (uint8_t i = 0; i < 16; ++i) { + for (uint8_t i = 0; i < max_serialized_key_length; ++i) { ihandle->value[i] = md5_.digest[i]; } } else { - for (uint8_t i = 0; i < 16; ++i) { - ihandle->value[i] = this->key_buffer_[i]; + memset(ihandle->value, 0, max_serialized_key_length); + for (uint8_t i = 0; i < ser_length; ++i) { + ihandle->value[i] = key_buffer_[i]; } }