Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attackers can crash whole node by panicking a single thread #5940

Closed
SWvheerden opened this issue Nov 10, 2023 · 0 comments · Fixed by #5943
Closed

Attackers can crash whole node by panicking a single thread #5940

SWvheerden opened this issue Nov 10, 2023 · 0 comments · Fixed by #5943
Assignees
Labels
release-blocker Something that needs to be fixed before a release can be made

Comments

@SWvheerden
Copy link
Collaborator

A panic on a thread will cause the whole process to be aborted. This happens both
in the wallet and in the base node.

fn main() {
// Setup a panic hook which prints the default rust panic message
but also exits the process. This makes a panic in
// any thread "crash" the system instead of silently continuing.
let default_hook = panic::take_hook();
panic::set_hook(Box::new(move |info| {
default_hook(info);
process::exit(1);
}));

This design decision allowed several vulnerabilities in this report to be of critical
severity. For example, panics on RPC requests would have practically no impact
when their thread panics instead of being a tool for triggering a denial of service on
the whole network.

Panics on threads should be treated as bugs and fixed, but an attacker would have
a much harder time converting them to successful exploits if the program did not
crash immediately.

@SWvheerden SWvheerden added the release-blocker Something that needs to be fixed before a release can be made label Nov 10, 2023
@SWvheerden SWvheerden self-assigned this Nov 10, 2023
SWvheerden added a commit that referenced this issue Nov 10, 2023
Description
---
Applications currently panic on any thread panic. Remove this so that
applications will not panic.

Motivation and Context
---
See: #5940 

This is somewhat debatable if this is desired or not behaviour. But
currently, the favoured approach is to only panic the thread.

Fixes: #5940
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Something that needs to be fixed before a release can be made
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant