Missing bounds check for collision object subframe names

Open Beginner friendly
  • Is something wrong?
#3,939 2 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

@huxiaoyi-ovo is already working on this.

Since Oct 30, 2026.

  • #3952 by @huxiaoyi-ovo — open
  • #3958 by @paneendrakumar0 — open

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
backend, robotics

Research direction

Read the two affected loops in moveit_core/planning_scene/src/planning_scene.cpp at the cited locations, then compare their validation with the check in moveit_core/robot_state/src/conversions.cpp around line 271. Add coverage for malformed subframe arrays in both PlanningScene paths; the issue suggests running the cases with AddressSanitizer or bounds checking. Done means both paths reject mismatched arrays without modifying the planning scene.

Written by the indexing model from the issue text.

Description

bug
Description

Two code paths in moveit_core/planning_scene/src/planning_scene.cpp iterate over subframe_poses.size() and access subframe_names[I] without checking whether the two vectors have equal lengths.

The affected locations are:

  • Lines 1616–1620: attached collision object processing.
  • Lines 1925–1933: collision object ADD processing.

The relevant code is:

for (std::size_t i = 0; i < object.subframe_poses.size(); ++i)
{
utilities::poseMsgToEigen(object.subframe_poses[i], subframe_pose);
std::string name = object.subframe_names[i];
subframes[name] = subframe_pose;
}

When subframe_poses.size() > subframe_names.size(), the access to subframe_names[i] is out of bounds, resulting in undefined behavior.

A similar code path in moveit_core/robot_state/src/conversions.cpp (around line 271) already performs an explicit size consistency check:

if (aco.object.subframe_poses.size() != aco.object.subframe_names.size())
{
RCLCPP_ERROR(...);
return;
}

The corresponding validation appears to be missing from the two PlanningScene code paths.

Suggested fix: Validate the lengths of subframe_poses and subframe_names before accessing either vector, and reject malformed messages without modifying the planning scene.

This issue is based on source-code inspection. Runtime reproduction is pending.

ROS Distro

Humble

OS and version

Ubuntu 22.04

Source or binary build?

Source

If binary, which release version?

No response

If source, which branch?

No response

Which RMW are you using?

None

Steps to Reproduce

Proposed reproduction steps :

  1. Create a moveit_msgs::msg::CollisionObject containing valid collision geometry.
  2. Add one valid pose to subframe_poses.
  3. Leave subframe_names empty.
  4. Submit the message through the PlanningScene collision object ADD processing path.
  5. Run the test with AddressSanitizer or a bounds-checking STL configuration to detect the potential out-of-bounds access.

The attached collision object path should be tested separately with the same mismatched subframe arrays.

Expected behavior

The operation should reject inconsistent subframe arrays and report an error without accessing memory outside the vector.

Actual behavior

Source inspection shows that the loop indexes subframe_names[i] without validating the vector length.This behavior may result in an out-of-bounds access. Runtime reproduction is still pending.

Backtrace or Console output

No response

Dominant language
C++
Stars
2.1k
Forks
816
Avg merge
1d 2h
Merged PRs (30d)
4

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from moveit/moveit2

All issues in moveit/moveit2

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.