ros / actionlib

Provides a standardized interface for interfacing with preemptable tasks. Examples of this include moving the base to a target location, performing a laser scan and returning the resulting point cloud, detecting the handle of a door, etc.
http://www.ros.org/wiki/actionlib
96 stars 155 forks source link

Address RVD#2401 #171

Closed vmayoral closed 4 years ago

vmayoral commented 4 years ago

Our team at @AliasRobotics identified and reported in RVD#2401 the use of unsafe yaml load (https://github.com/aliasrobotics/RVD/issues/2401).

After triaging the flaw we detected that it was exploitable and could lead to local (or remote, based on certain common user interaction) code execution.

Specifically, the flaw itself is caused by an unsafe parsing of YAML values which happens whenever an action message is processed to be sent, and allows for the creation of Python objects. Through this flaw in ROS, an attacker could build a malicious payload and execute arbitrary code in Python. A PoC is available but have decided not to disclose it for now and until this is mitigated and debs are available.

Peer-researched and coded with @ibaiape.

mikaelarguedas commented 4 years ago

+1 I had to add the same fix as part of https://github.com/ros/actionlib/pull/169

vmayoral commented 4 years ago

Thanks for the quick reaction @mjcarroll. Note that in https://github.com/aliasrobotics/RVD/issues/2401#issuecomment-677393754 we describe a number of other packages you guys might want to take a look at and which present a similar issue. We haven't had time to look into all them but I don't see any reason why we couldn't just use secure loading instead directly.

When should we expect new deb files?

mjcarroll commented 4 years ago

I'm doing a release into melodic and noetic right now. After discussion with rosboss @clalancette, it sounds like it may make it into the pending melodic sync https://discourse.ros.org/t/preparing-for-melodic-sync-2020-08-19/15988

https://github.com/ros/rosdistro/pull/26269

vmayoral commented 4 years ago

I'm doing a release into melodic and noetic right now. After discussion with rosboss @clalancette, it sounds like it may make it into the pending melodic sync https://discourse.ros.org/t/preparing-for-melodic-sync-2020-08-19/15988

Cool, that's a fast response!

jspricke commented 4 years ago

Hi, thanks for looking into ROS security! As far as I can see the code in questions is only used in axclient/axserver and only with data from the user running the process. Can you share your PoC with me? You can find my address on my profile. Thanks!

vmayoral commented 4 years ago

Hey @jspricke, you're right, it's used in axclient/server. As pointed out above, as far as we've looked into this so far, remote code execution is possible based on some user interaction but we'd argue it should be feasible to escalate privileges with a different exploit. We haven't built this second one due to resource limitations yet.

I'm finalizing a report now and will include there details of the PoC. Ping me in a couple of weeks if I haven't disclosed it and I'll look into facilitating the report to you beforehand.

vmayoral commented 4 years ago

ping @jspricke, you'll find the report disclosed at https://discourse.ros.org/t/red-teaming-ros-and-ros-industrial-packages/16448

mgrrx commented 4 years ago

I honestly still don't see an issue here. You are User1 on Host1, have shell access and start a program called axclient and this runs As PIDX. With "remote code execution" you refer to running code within this PID X, not in any other program. This is not remote code execution, but local code execution as the code that you inject in yaml.load runs on Host1 as User1 inside PID X. This code is executed with exactly the same set of privileges.

mgrrx commented 4 years ago

I assume there are dozens of other security holes in ROS but this is certainly not worth a CVE.

vmayoral commented 4 years ago

Hi @mgrrx, when i opened this ticket I explicitly said:

could lead to local (or remote, based on certain common user interaction) code execution

Depending on the assumptions and user interaction, as pointed out. Your assumptions and setup might be different from others’. Refer to our article for more details on how we looked at this. You may reconsider some of the conclusions you’ve got now then.

I assume there are dozens of other security holes in ROS but this is certainly not worth a CVE.

Edited: A CVE was granted and approved by MITRE on this issue according to the PoC we presented but criticism and rebuttals to the assignation are welcome. I presume what you mean to argue is not the qualification itself as a CVE but more the score assigned based on our assessment. Feel free to jump into https://github.com/aliasrobotics/RVD/issues/2401 and share your views please.

mgrrx commented 4 years ago

I have read the article and still don't see an issue here. As I said you will be able to execute code locally but as an attacker you should simply not start the axclient if you are already that far, sorry.