make_serialized_message() uses memcpy without checking the size of the input buffer, which may lead to out-of-bounds reads

[zhihaoshang]

Description

rosbag2_storage::make_serialized_message() does not verify the actual readable length of the input pointer data. When the provided size is greater than the valid memory size pointed to by data, it can lead to out-of-bounds reading in memcpy.

Expected Behavior

Safe return, no out-of-bounds access

Actual Behavior

Out-of-bounds read

To Reproduce

Test Case

#include <gmock/gmock.h>
#include <memory>
#include "rosbag2_storage/ros_helper.hpp"
using namespace ::testing;  // NOLINT
TEST(ros_helper, make_serialized_message_buffer_overflow) {
  double data_value = 3.14;
  auto data = new double;
  *data = data_value;
  auto size = sizeof(double) + 1; 
  ASSERT_NO_THROW({
    auto serialized_message = rosbag2_storage::make_serialized_message(data, size);
    ASSERT_THAT(serialized_message->buffer_length, Eq(0u)); 
    ASSERT_THAT(serialized_message->buffer_capacity, Eq(size));
  });
  delete data;
}

Output

[ RUN      ] ros_helper.make_serialized_message_buffer_overflow
=================================================================
==7602==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x502000009e18 at pc 0x77e66b0fb42e bp 0x7fff18d7d290 sp 0x7fff18d7ca38
READ of size 9 at 0x502000009e18 thread T0
    #0 0x77e66b0fb42d in memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc:115
    #1 0x77e66af62421 in rosbag2_storage::make_serialized_message(void const*, unsigned long) (/home/shangzh/ros2_jazzy/install/rosbag2_storage/lib/librosbag2_storage.so+0x140421) (BuildId: 9356fba64f173b3892930a1219d31e451df57aea)
    #2 0x5b11c0256587 in ros_helper_make_serialized_message_buffer_overflow_Test::TestBody() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xaf587) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #3 0x5b11c02fb55b in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x15455b) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #4 0x5b11c02e8b5d in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x141b5d) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #5 0x5b11c028f17f in testing::Test::Run() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xe817f) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #6 0x5b11c029095b in testing::TestInfo::Run() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xe995b) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #7 0x5b11c0291cb4 in testing::TestSuite::Run() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xeacb4) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #8 0x5b11c02b89df in testing::internal::UnitTestImpl::RunAllTests() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x1119df) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #9 0x5b11c02fe9b6 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x1579b6) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #10 0x5b11c02ebdfa in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x144dfa) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #11 0x5b11c02b4fe9 in testing::UnitTest::Run() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x10dfe9) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #12 0x5b11c025f48a in RUN_ALL_TESTS() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xb848a) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #13 0x5b11c025f3d6 in main (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xb83d6) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #14 0x77e66902a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #15 0x77e66902a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #16 0x5b11c0256234 in _start (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xaf234) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)

0x502000009e18 is located 0 bytes after 8-byte region [0x502000009e10,0x502000009e18)
allocated by thread T0 here:
    #0 0x77e66b0fe548 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x5b11c025644d in ros_helper_make_serialized_message_buffer_overflow_Test::TestBody() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xaf44d) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #2 0x5b11c02fb55b in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x15455b) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #3 0x5b11c02e8b5d in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x141b5d) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #4 0x5b11c028f17f in testing::Test::Run() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xe817f) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #5 0x5b11c029095b in testing::TestInfo::Run() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xe995b) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #6 0x5b11c0291cb4 in testing::TestSuite::Run() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xeacb4) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #7 0x5b11c02b89df in testing::internal::UnitTestImpl::RunAllTests() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x1119df) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #8 0x5b11c02fe9b6 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x1579b6) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #9 0x5b11c02ebdfa in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x144dfa) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #10 0x5b11c02b4fe9 in testing::UnitTest::Run() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0x10dfe9) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #11 0x5b11c025f48a in RUN_ALL_TESTS() (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xb848a) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #12 0x5b11c025f3d6 in main (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xb83d6) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)
    #13 0x77e66902a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #14 0x77e66902a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #15 0x5b11c0256234 in _start (/home/shangzh/rosbag2_ws/build/rosbag2_storage/test_ros_helper+0xaf234) (BuildId: 5275504d263dde7b31b728b1190be016e97599fb)

SUMMARY: AddressSanitizer: heap-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc:115 in memcpy
Shadow bytes around the buggy address:
  0x502000009b80: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fd
  0x502000009c00: fa fa fd fa fa fa fd fd fa fa 00 fa fa fa 00 00
  0x502000009c80: fa fa 00 fa fa fa 00 fa fa fa 00 fa fa fa 00 fa
  0x502000009d00: fa fa 00 fa fa fa 04 fa fa fa 00 fa fa fa 04 fa
  0x502000009d80: fa fa 00 fa fa fa fd fa fa fa 00 fa fa fa 00 00
=>0x502000009e00: fa fa 00[fa]fa fa 00 01 fa fa fa fa fa fa fa fa
  0x502000009e80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x502000009f00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x502000009f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x50200000a000: fa fa 00 00 fa fa fa fa fa fa fa fa fa fa fa fa
  0x50200000a080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==7602==ABORTING

System (please complete the following information)

OS: ubuntu 24.04
ROS 2 Distro: ros 2 jazzy
Install Method: source
Version: ros 2 jazzy
build options: --mixin asan-gcc

[fujitatomoya]

@zhihaoshang how do you suggest to fix this? the application should be responsible for knowing and managing the size/capacity alongside the raw pointer. please consider pull request.