Skip to content

Crash due to Unhandled Null Pointer in ParameterEventsFilter Constructor #2810

@zhihaoshang

Description

@zhihaoshang

Operating System:

Linux shangzh-VMware-Virtual-Platform 6.11.0-21-generic #21~24.04.1-Ubuntu SMP PREEMPT_DYNAMIC Mon Feb 24 16:52:15 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

ROS version or commit hash:

ros2 jazzy

RMW implementation (if applicable):

No response

RMW Configuration (if applicable):

No response

Client library (if applicable):

rclcpp

'ros2 doctor --report' output

ros2 doc --report
   NETWORK CONFIGURATION
inet         : 127.0.0.1
inet4        : ['127.0.0.1']
inet6        : ['::1']
netmask      : 255.0.0.0
device       : lo
flags        : 73<RUNNING,UP,LOOPBACK>
mtu          : 65536
inet         : 192.168.148.137
inet4        : ['192.168.148.137']
ether        : 00:0c:29:be:c8:19
inet6        : ['fe80::20c:29ff:febe:c819%ens33']
netmask      : 255.255.255.0
device       : ens33
flags        : 4163<RUNNING,UP,BROADCAST,MULTICAST>
mtu          : 1500
broadcast    : 192.168.148.255

   PLATFORM INFORMATION
system           : Linux
platform info    : Linux-6.11.0-21-generic-x86_64-with-glibc2.39
release          : 6.11.0-21-generic
processor        : x86_64

   QOS COMPATIBILITY LIST
compatibility status    : No publisher/subscriber pairs found

   RMW MIDDLEWARE
middleware name    : rmw_fastrtps_cpp

   TOPIC LIST
topic               : none
publisher count     : 0
subscriber count    : 0

Steps to reproduce issue

Environment

OS Version: Ubuntu 24.04
rclcpp version: ros2 jazzy
Compiler name and version number: Ubuntu clang version 18.1.3
Source or binary build?
source build
build options: --mixin asan-gcc

Description

When using the ParameterEventsFilter constructor, passing a null pointer (nullptr) causes the program to crash. This issue occurs because the constructor does not check for null pointers, leading to a crash when attempting to dereference the null pointer.

Test Case

#include <gtest/gtest.h>
#include <string>
#include <memory>
#include "rclcpp/exceptions.hpp"
#include "rclcpp/parameter_events_filter.hpp"
#include "rcl_interfaces/msg/parameter_event.hpp"

class TestParameterEventFilter : public ::testing::Test
{
protected:
  void SetUp()
  {
    empty = std::make_shared<rcl_interfaces::msg::ParameterEvent>();
    full = std::make_shared<rcl_interfaces::msg::ParameterEvent>();
    multiple = std::make_shared<rcl_interfaces::msg::ParameterEvent>();
    np = std::make_shared<rcl_interfaces::msg::ParameterEvent>();
    cp = std::make_shared<rcl_interfaces::msg::ParameterEvent>();
    dp = std::make_shared<rcl_interfaces::msg::ParameterEvent>();
    rcl_interfaces::msg::Parameter p;
    p.name = "new";
    p.value.type = rcl_interfaces::msg::ParameterType::PARAMETER_INTEGER;
    p.value.integer_value = 1;
    full->new_parameters.push_back(p);
    np->new_parameters.push_back(p);
    multiple->new_parameters.push_back(p);
    p.name = "new2";
    p.value.integer_value = 2;
    multiple->new_parameters.push_back(p);
    p.name = "changed";
    p.value.integer_value = 1;
    full->changed_parameters.push_back(p);
    cp->changed_parameters.push_back(p);
    p.name = "deleted";
    p.value.integer_value = 1;
    full->deleted_parameters.push_back(p);
    dp->changed_parameters.push_back(p);
  }

  rcl_interfaces::msg::ParameterEvent::SharedPtr empty;
  rcl_interfaces::msg::ParameterEvent::SharedPtr full;
  rcl_interfaces::msg::ParameterEvent::SharedPtr multiple;
  rcl_interfaces::msg::ParameterEvent::SharedPtr np;
  rcl_interfaces::msg::ParameterEvent::SharedPtr cp;
  rcl_interfaces::msg::ParameterEvent::SharedPtr dp;
  rclcpp::ParameterEventsFilter::EventType nt = rclcpp::ParameterEventsFilter::EventType::NEW;
  rclcpp::ParameterEventsFilter::EventType ct = rclcpp::ParameterEventsFilter::EventType::CHANGED;
  rclcpp::ParameterEventsFilter::EventType dt = rclcpp::ParameterEventsFilter::EventType::DELETED;
};

TEST_F(TestParameterEventFilter, null_event_pointer) {
  rcl_interfaces::msg::ParameterEvent::SharedPtr null_ptr = nullptr;
  EXPECT_THROW(
    rclcpp::ParameterEventsFilter(null_ptr, {"new"}, {nt}),
    std::invalid_argument
  );
}

Output

[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from TestParameterEventFilter
[ RUN      ] TestParameterEventFilter.null_event_pointer
AddressSanitizer:DEADLYSIGNAL
=================================================================
==252400==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000028 (pc 0x79bdec9a5bcb bp 0x7ffc7d2787e0 sp 0x7ffc7d2787d0 T0)
==252400==The signal is caused by a READ memory access.
==252400==Hint: address points to the zero page.
    #0 0x79bdec9a5bcb in __gnu_cxx::__normal_iterator<rcl_interfaces::msg::Parameter_<std::allocator<void> > const*, std::vector<rcl_interfaces::msg::Parameter_<std::allocator<void> >, std::allocator<rcl_interfaces::msg::Parameter_<std::allocator<void> > > > >::__normal_iterator(rcl_interfaces::msg::Parameter_<std::allocator<void> > const* const&) (/home/shangzh/ros2_jazzy/install/rclcpp/lib/librclcpp.so+0xda5bcb) (BuildId: 6fc5c95245c12254f84b0fb758e76bec499949f1)
    #1 0x79bdec99b250 in std::vector<rcl_interfaces::msg::Parameter_<std::allocator<void> >, std::allocator<rcl_interfaces::msg::Parameter_<std::allocator<void> > > >::begin() const (/home/shangzh/ros2_jazzy/install/rclcpp/lib/librclcpp.so+0xd9b250) (BuildId: 6fc5c95245c12254f84b0fb758e76bec499949f1)
    #2 0x79bdecac58f2 in rclcpp::ParameterEventsFilter::ParameterEventsFilter(std::shared_ptr<rcl_interfaces::msg::ParameterEvent_<std::allocator<void> > const>, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::vector<rclcpp::ParameterEventsFilter::EventType, std::allocator<rclcpp::ParameterEventsFilter::EventType> > const&) (/home/shangzh/ros2_jazzy/install/rclcpp/lib/librclcpp.so+0xec58f2) (BuildId: 6fc5c95245c12254f84b0fb758e76bec499949f1)
    #3 0x6020b485a4e8 in TestParameterEventFilter_null_event_pointer_Test::TestBody() (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0x964e8) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #4 0x6020b4909217 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0x145217) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #5 0x6020b48f6a21 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0x132a21) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #6 0x6020b489cba5 in testing::Test::Run() (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0xd8ba5) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #7 0x6020b489e381 in testing::TestInfo::Run() (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0xda381) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #8 0x6020b489f6da in testing::TestSuite::Run() (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0xdb6da) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #9 0x6020b48c6405 in testing::internal::UnitTestImpl::RunAllTests() (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0x102405) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #10 0x6020b490c672 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0x148672) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #11 0x6020b48f9cbe in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0x135cbe) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #12 0x6020b48c2a0f in testing::UnitTest::Run() (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0xfea0f) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #13 0x6020b486ceb1 in RUN_ALL_TESTS() (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0xa8eb1) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #14 0x6020b486cdfd in main (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0xa8dfd) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)
    #15 0x79bdeae2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #16 0x79bdeae2a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #17 0x6020b485a0b4 in _start (/home/shangzh/ros2_jazzy/build/rclcpp/test/rclcpp/test_parameter_events_filter+0x960b4) (BuildId: 574b4520414cc61b5fc47f14a7898e117fe3790b)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/home/shangzh/ros2_jazzy/install/rclcpp/lib/librclcpp.so+0xda5bcb) (BuildId: 6fc5c95245c12254f84b0fb758e76bec499949f1) in __gnu_cxx::__normal_iterator<rcl_interfaces::msg::Parameter_<std::allocator<void> > const*, std::vector<rcl_interfaces::msg::Parameter_<std::allocator<void> >, std::allocator<rcl_interfaces::msg::Parameter_<std::allocator<void> > > > >::__normal_iterator(rcl_interfaces::msg::Parameter_<std::allocator<void> > const* const&)
==252400==ABORTING

Expected behavior

When the ParameterEventsFilter constructor receives a null pointer (nullptr) as a parameter, it should throw a std::invalid_argument exception, rather than dereferencing the null pointer and causing a memory access error.

Actual behavior

When a null pointer is passed, the ParameterEventsFilter constructor does not check for the null pointer, resulting in a crash (SEGV error) when attempting to dereference the null pointer during operations like std::find or accessing parameters.

Additional information

Suggested Fix

It is recommended to add a null pointer check at the beginning of the ParameterEventsFilter constructor. If the event parameter is null, throw a std::invalid_argument exception to prevent further memory access errors.

if (!event) {
  throw std::invalid_argument("ParameterEvent cannot be null");
}

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions