eclipse-cyclonedds / cyclonedds

Eclipse Cyclone DDS project
https://projects.eclipse.org/projects/iot.cyclonedds
Other
876 stars 358 forks source link

About Vulnerabilities in HeartBeat Messages [QoS - Reliable Mode] #1854

Open Desglaneurs opened 1 year ago

Desglaneurs commented 1 year ago

In CycloneDDS in ROS2, a vulnerability was discovered a malicious attacker manipulates the sequence of Heartbeat packets to interfere with communication between Publisher & Subscriber. Manipulating the sequence of an existing Heartbeat packet to a large value that has not yet been reached can lead to a Denial of Service (DoS) condition in the system. Related details are as shown in [Link]

eboasson commented 1 year ago

Thank you for reporting this and thank you for the care you put in putting together the description.

This is a known weakness of the spec'd protocol. Unfortunately the ability of the reliable protocol to skip sequence numbers is fundamental to the some of the combinations of QoS settings in DDS and so it is not a matter of a simple bug fix (like the one you kindly suggested). I have at times wondered what I could do mitigate this weakness within the confines of the spec—or what one could do outside it—but it is not at all straightforward.

The only thing that I have come up with so far within the protocol that doesn't involve heuristics is to not blindly accept the Heartbeat message but instead to request a retransmit of the "missing" sequence numbers and see what happens. The protocol has a mechanism for a writer to report that these messages no longer exist and so this would mean a single crafted Heartbeat message can't cause this anymore. However, it doesn't seem to be worth the effort because if you can send a crafted Heartbeat message, then surely you can also spoof the rest of the protocol without too much trouble.

The only real mitigation that I am aware of involves DDS Security. That allows you to check message authenticity and integrity and this trick no longer works so easily. Unfortunately, it comes with significant overhead.

I have a few more remarks.

The first is that it is great to see you using the RTPS packet support in scapy: it was done by friends of mine for a study into DDS vulnerabilities and it always nice to see things being adopted more widely 😀

The second is that security vulnerabilities are better reported privately first, rather than openly on GitHub so that a fix or mitigation can be put into place before it is made public. I now realize that Cyclone DDS is missing a SECURITY file in the top of the repository detailing how to do that and I assume you created the issue on GitHub because you did not see another option.

For any Eclipse project you can always contact the Eclipse Foundation Security Team, and each project also has a public list of committers and project leads. Contacting any of those directly would be a reasonable choice as well (in the case of Cyclone ... well, I'm easy to find!) I would be grateful if you considered these options for anything else you find.

And last, addressing an issue like this likely requires specification changes and coordinated changes in many implementations. For that reason, I think it would make the most sense for you to raise it with the OMG instead of with various implementations, because the OMG is the standards body for the DDS specifications and it provides you with a single access point.

Desglaneurs commented 1 year ago

But we think of this as a clear security issue. The reason for this is that the CVE number does not exist, and there is no document mentioned. So, we would like to inform the security issue of ROS2 from the standpoint of using open source by registering and notifying CVE.

eboasson commented 1 year ago

I don't believe you are correct in considering this a security issue in itself: it is not fundamentally different from spoofing data. Even if one were to have an oracle that rejects crafted Heartbeat messages, all it would take to have the same effect is generating large amounts of nonsense with increasing sequence numbers.

There are countless ways in which an unprotected DDS system can be manipulated in accepting invalid application data or made to skip valid messages. This is simply one of them. I don't like the existence of so many ways of doing that, and I would like mitigations that would reduce the likelihood, but for a system that operates entirely in plain text and without authenticating messages this is par for the course.

Indeed, one should use protection measures unless one has determined that in the particular cases the risks of not doing so are acceptable; the default should be to use the measures available. What I accept as an issue is that no documentation is provided that would help a user determine the risks. I know there is awareness of the general risks in the ROS2 community and that that is why SROS2 is a thing.

If you can do the same while DDS Security is used to protect the integrity of the system, then it is a very serious vulnerability indeed.