Potential vulnerability: topic and service names from ROS parameters

[tandf]

Hi,

We notice that you are using topic and service names from ROS parameters at the following locations:

grid_map/grid_map_demos/scripts/image_publisher.py

Line 57 in 03f8676

publisher = rospy.Publisher(topicName, sensor_msgs.msg.Image, queue_size=10)

grid_map/grid_map_demos/src/FiltersDemo.cpp

Line 25 in 03f8676

subscriber_ = nodeHandle_.subscribe(inputTopic_, 1, &FiltersDemo::callback, this);

grid_map/grid_map_demos/src/FiltersDemo.cpp

Line 26 in 03f8676

publisher_ = nodeHandle_.advertise<grid_map_msgs::GridMap>(outputTopic_, 1, true);

grid_map/grid_map_demos/src/GridmapToImageDemo.cpp

Line 20 in 03f8676

gridMapSubscriber_ = nodeHandle_.subscribe(gridMapTopic_, 1, &GridMapToImageDemo::gridMapCallback, this);

grid_map/grid_map_demos/src/ImageToGridmapDemo.cpp

Line 20 in 03f8676

imageSubscriber_ = nodeHandle_.subscribe(imageTopic_, 1, &ImageToGridmapDemo::imageCallback, this);

grid_map/grid_map_demos/src/OctomapToGridmapDemo.cpp

Line 25 in 03f8676

client_ = nodeHandle_.serviceClient<octomap_msgs::GetOctomap>(octomapServiceTopic_);

grid_map/grid_map_demos/src/SdfDemo.cpp

Line 20 in 03f8676

pointcloudPublisher_ = nodeHandle.advertise<sensor_msgs::PointCloud2>(pointcloudTopic + "/full_sdf", 1);

grid_map/grid_map_demos/src/SdfDemo.cpp

Line 21 in 03f8676

freespacePublisher_ = nodeHandle.advertise<sensor_msgs::PointCloud2>(pointcloudTopic + "/free_space", 1);

grid_map/grid_map_demos/src/SdfDemo.cpp

Line 22 in 03f8676

occupiedPublisher_ = nodeHandle.advertise<sensor_msgs::PointCloud2>(pointcloudTopic + "/occupied_space", 1);

grid_map/grid_map_demos/src/SdfDemo.cpp

Line 23 in 03f8676

gridmapSubscriber_ = nodeHandle.subscribe(mapTopic, 1, &SdfDemo::callback, this);

grid_map/grid_map_loader/src/GridMapLoader.cpp

Line 21 in 03f8676

publisher_ = nodeHandle_.advertise<grid_map_msgs::GridMap>(publishTopic_, 1, true);

grid_map/grid_map_visualization/src/GridMapVisualization.cpp

Line 147 in 03f8676

mapSubscriber_ = nodeHandle_.subscribe(mapTopic_, 1, &GridMapVisualization::callback, this);

For security reasons detailed below, we strongly suggest avoiding the usage of strings from parameters as topic and service names.

Although parameters are usually set in parameter files, they can also be changed by nodes. Specifically, other nodes in the same ROS application can also change the parameters listed above before it’s used, either by accident or intentionally (i.e., by potential attackers). For example, if the grid_map_topic parameter is changed, the grid_map_to_image_demo node may therefore subscribe to a wrong topic, and will not be able to save anything to the image file. Moreover, if an attacker exists, she can control the image content by first fooling the grid_map_to_image_demo node to subscribe to another topic like /grid_map_fake, and then forwarding the messages from /grid_map to /grid_map_fake after changing the contents as she wants to. Because ROS is an OSS (open-source software) community, third-party nodes are widely used in ROS applications, usually without complete vetting of their behavior, which gives the opportunity to potentially malicious actors to inject malicious code (e.g, by submitting hypocrite commits like in other OSS systems [1]) to infiltrate the ROS applications that use it (or software supply chain attacks, one of the primary means for real-world attackers today [2]).

We understand that using parameters to set topic and service names brings flexibility. Still, for the purpose of security, we strongly suggest that you avoid such vulnerable programming patterns if possible. For example, to avoid the exposure of this specific vulnerability, you may consider alternatives like remapping, which is designed for configuring names when launching the nodes.

[1] Q. Wu and K. Lu, “On the feasibility of stealthily introducing vulnerabilities in open-source software via hypocrite commits,” 2021, https://linuxreviews.org/images/d/d9/OpenSourceInsecurity.pdf.
[2] Supply chain attacks are the hacker’s new favourite weapon. and the threat is getting bigger. https://www.zdnet.com/article/supply-chain-attacks-are-the-hackers-new-favourite-weapon-and-the-threat-is-getting-bigger/.

[tandf]

Hi there, I wanted to follow up on this security vulnerability. Could you please let me know if there have been any updates or concerns regarding this issue? Thanks

[Ryanf55]

Hey there,

ROS 2 has two methods that invalidate this.

1. Use SROS2 to secure the topics: https://github.com/ros2/sros2
2. Use ROS_LOCALHOST_ONLY if you don't have communications between boxes
   https://discourse.ros.org/t/unconfigured-dds-considered-harmful-to-networks/25689

Anyone that leaves their ROS nodes open to a network are going to have larger problems beyond topic names.

For example, even with a static topic, you can crash a node by publishing the wrong type on the topic. This is a known vulnerability.

That said, I would be more than happy to accept a PR that removes the parameters and then adds instructions for remapping them, which from what I understand, is the preferred way.