parallelchain-io / hotstuff_rs

Rust implementation of the HotStuff consensus algorithm.
34 stars 4 forks source link

Size of progress message stub buffer is unbounded #1

Closed ghost closed 6 months ago

ghost commented 1 year ago

A potential vulnerability related to message caching exists in networking.rs.

The vulnerability is in the following code block in recv():

// Cache the message if its for a future view.
else if msg.view() > cur_view {
    let msg_queue = if let Some(msg_queue) = self.msg_buffer.get_mut(&cur_view) {
        msg_queue
    } else {
        self.msg_buffer.insert(cur_view, VecDeque::new());
        self.msg_buffer.get_mut(&cur_view).unwrap()
    };
    msg_queue.push_back((sender, msg));
}

The issue lies in the condition

else if msg.view() > cur_view,

which allows messages with a view greater than the current view to be cached.

The problem is that there is no limit or validation on how large the difference between the current view and the message's view can be. This could potentially lead to an unbounded growth of the msg_buffer map. An attacker can exploit this vulnerability by sending messages with a continuously increasing view number that is much larger than the current view. This would cause the msg_buffer map to grow indefinitely, consuming more and more memory over time. Eventually, the system could run out of memory, resulting in a denial-of-service (DoS) situation. To mitigate this vulnerability, you should add a validation step to check the difference between the current view and the message's view and enforce a reasonable limit. If the difference exceeds the limit, you should either drop the message immediately or handle it in an appropriate manner.

ghost commented 7 months ago

Why is the name of the task changed from Denial of service vulnerability in networking.rs to something else? Changing title names does not solve the issue.

lyulka commented 6 months ago

We like to keep our issue names (and our comments in general) specific. There are countless kinds of potential DoS vulnerabilities in software systems in general. The current name of this issue specifies precisely the kind of potential DoS vulnerability that HotStuff-rs was susceptible to in v0.2.x.

This issue has been resolved in the v0.3.0 release.

ghost commented 6 months ago

I don’t see how this was solved. When the issue is closed, I would expect a comment or a summary to the solution before closure. How can I expect to trust your code when there is little to no credibility in the team and no communication.