skasperski / navigation_2d

ROS nodes to navigate a mobile robot in a planar environment
GNU General Public License v3.0
123 stars 65 forks source link

Potential vulnerability: topic and service names from ROS parameters #65

Open tandf opened 1 year ago

tandf commented 1 year ago

Hi,

We notice that you are using topic and service names from ROS parameters, e.g. at the following locations: https://github.com/skasperski/navigation_2d/blob/noetic/nav2d_navigator/src/RobotNavigator.cpp#L22 https://github.com/skasperski/navigation_2d/blob/noetic/nav2d_localizer/src/LocalizerNode.cpp#L19 https://github.com/skasperski/navigation_2d/blob/noetic/nav2d_karto/src/MultiMapper.cpp#L41 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). Such changes can cause various results from disabling localization and move functionality to generating wrong localization results and move commands that potentially lead to crashes. For example, if the parameter map_service is changed, the RobotNavigator node will try to get map from a wrong service as written in here: https://github.com/skasperski/navigation_2d/blob/noetic/nav2d_navigator/src/RobotNavigator.cpp#L22 If an attacker exists, she can deliberately fool the navigator node to get map from a wrong service hosted by the attacker, and maliciously modified map will be used by the navigator. The same name-from-parameter pattern is also used for move related purposes, like here: https://github.com/skasperski/navigation_2d/blob/noetic/nav2d_navigator/src/RobotNavigator.cpp#L51. If move action is controlled by a malicious node, the attacker obtains the control capability of the robot and can move it to anywhere as she wants to. The same vulnerabilities also exist in the code for localizer and mapper, as listed at the beginning. 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/.

skasperski commented 1 year ago

Hi,

thank you for looking into this and raising awareness on the issue. While I would generally advise against using this kind of educational software in any safety-critical application, I can see the point that not every user of ROS packages might be fully aware of this. I will have a look at the suggested changes.