From 261a46772f721b62aed6228fb03c1cb8417c6083 Mon Sep 17 00:00:00 2001 From: Ethan Gao <16472154+gaoethan@users.noreply.github.com> Date: Thu, 14 Dec 2017 00:31:18 +0800 Subject: [PATCH] segmentation fault for NULL dereference (#202) * Invalid memory access for NULL dereference rcl_lifecycle_get_state may return NULL transition->goal is checked for NULL but no return while it's true and this may result the follow-up NULL dereference Signed-off-by: Ethan Gao * Avoid crash while NULL returned from rcl_service_get_options() Signed-off-by: Ethan Gao * tweak error string and null check for current_state Signed-off-by: Ethan Gao --- rcl/src/rcl/service.c | 4 ++++ rcl_lifecycle/src/rcl_lifecycle.c | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/rcl/src/rcl/service.c b/rcl/src/rcl/service.c index 2df5b0125..a3c606f53 100644 --- a/rcl/src/rcl/service.c +++ b/rcl/src/rcl/service.c @@ -247,6 +247,8 @@ rcl_take_request( { RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Service server taking service request") const rcl_service_options_t * options = rcl_service_get_options(service); + RCL_CHECK_FOR_NULL_WITH_MSG( + options, "Failed to get service options", return RCL_RET_ERROR, rcl_get_default_allocator()); RCL_CHECK_ARGUMENT_FOR_NULL(request_header, RCL_RET_INVALID_ARGUMENT, options->allocator); RCL_CHECK_ARGUMENT_FOR_NULL(ros_request, RCL_RET_INVALID_ARGUMENT, options->allocator); @@ -273,6 +275,8 @@ rcl_send_response( { RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Sending service response") const rcl_service_options_t * options = rcl_service_get_options(service); + RCL_CHECK_FOR_NULL_WITH_MSG( + options, "Failed to get service options", return RCL_RET_ERROR, rcl_get_default_allocator()); RCL_CHECK_ARGUMENT_FOR_NULL(request_header, RCL_RET_INVALID_ARGUMENT, options->allocator); RCL_CHECK_ARGUMENT_FOR_NULL(ros_response, RCL_RET_INVALID_ARGUMENT, options->allocator); diff --git a/rcl_lifecycle/src/rcl_lifecycle.c b/rcl_lifecycle/src/rcl_lifecycle.c index 6c9e30b6b..824610132 100644 --- a/rcl_lifecycle/src/rcl_lifecycle.c +++ b/rcl_lifecycle/src/rcl_lifecycle.c @@ -285,15 +285,20 @@ rcl_lifecycle_is_valid_transition( const rcl_lifecycle_state_t * current_state = rcl_lifecycle_get_state( &state_machine->transition_map, current_id); + RCL_CHECK_FOR_NULL_WITH_MSG(current_state, + "rcl_lifecycle_get_state returns NULL", return NULL, rcl_get_default_allocator()); + for (unsigned int i = 0; i < current_state->valid_transition_size; ++i) { if (current_state->valid_transition_keys[i] == key) { return ¤t_state->valid_transitions[i]; } } + RCUTILS_LOG_WARN_NAMED( ROS_PACKAGE_NAME, "No callback transition matching %d found for current state %s", key, state_machine->current_state->label) + return NULL; } @@ -318,6 +323,7 @@ rcl_lifecycle_trigger_transition( if (!transition->goal) { RCUTILS_LOG_ERROR_NAMED( ROS_PACKAGE_NAME, "No valid goal is set") + return RCL_RET_ERROR; } state_machine->current_state = transition->goal; if (publish_notification) {