hrnr / m-explore

ROS packages for multi robot exploration
Other
323 stars 210 forks source link

Potential vulnerability: topic names from ROS parameters #74

Open tandf opened 1 year ago

tandf commented 1 year ago

Hi,

We notice that you are using topic names from ROS parameters at the following locations: https://github.com/hrnr/m-explore/blob/fe0e0f3bf91c3da3fc46d3561a24b98692ba8c04/explore/src/costmap_client.cpp#L68 https://github.com/hrnr/m-explore/blob/fe0e0f3bf91c3da3fc46d3561a24b98692ba8c04/explore/src/costmap_client.cpp#L81 https://github.com/hrnr/m-explore/blob/06de4b609d002ca3111e7226ddb7fa08e058b202/map_merge/src/map_merge.cpp#L67 https://github.com/hrnr/m-explore/blob/06de4b609d002ca3111e7226ddb7fa08e058b202/map_merge/src/map_merge.cpp#L127 https://github.com/hrnr/m-explore/blob/06de4b609d002ca3111e7226ddb7fa08e058b202/map_merge/src/map_merge.cpp#L135 For security reasons detailed below, we strongly suggest avoiding the usage of strings from parameters as topic 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). Such changes can lead to denial-of-service and even man-in-the-middle attacks. In the map_merge case, both the input and output map topics are configured through parameters, which means changes to these parameters may lead to interruption of service, and the downstream applications cannot receive a merged map. If an attacker exists, she can even first fool the map_merger to publish to a fake topic e.g. /map_fake, then sending a modified map based on the map from topic /map_fake to topic /map that is actually used by downstream applications. Considering the functionality of downstream applications, the victim robots may fail to navigate, or even crash into obstacles, just as the attacker wants. Similarly, for explore_lite package, changes to ~/costmap_topic and ~/costmap_updates_topic parameters can lead to failure of navigation and even crashes. 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 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 commented 1 year ago

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