Skip to content

Fix slasher OOM#9141

Merged
mergify[bot] merged 4 commits intosigp:unstablefrom
michaelsproul:slasher-fix-w-test
Apr 20, 2026
Merged

Fix slasher OOM#9141
mergify[bot] merged 4 commits intosigp:unstablefrom
michaelsproul:slasher-fix-w-test

Conversation

@michaelsproul
Copy link
Copy Markdown
Member

Issue Addressed

Fix a vulnerability in the slasher whereby it would OOM upon processing an invalid attestation with an artificially high validator_index. This fix has already been made available to affected users on the slasher-fix branch.

Proposed Changes

  • Prevent attestations from being passed to the slasher prior to signature verification. This was unnecessary, as they would later be passed on successful validation as well.
  • Add a defensive cap on the maximum validator index processable by the slasher. The cap is high enough that it shouldn't be reached for several years, and will quickly result in warning logs if forgotten.
  • Add a regression test that confirms that the issue is fixed.

@michaelsproul michaelsproul added bug Something isn't working ready-for-review The code is ready for review slasher labels Apr 16, 2026
Comment thread slasher/src/attestation_queue.rs Outdated
validator_index,
"Dropping slasher attestation with out-of-range validator index"
);
continue;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This loop is in acending validator_index right? would we want to break here if it happens?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Yeah it's a BTreeMap so a break makes sense

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in:

Copy link
Copy Markdown
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and nice test!
Just a minor question on continue.

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 20, 2026
@mergify mergify Bot added the queued label Apr 20, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 20, 2026

Merge Queue Status

This pull request spent 26 minutes 38 seconds in the queue, including 26 minutes 19 seconds running CI.

Required conditions to merge

mergify Bot added a commit that referenced this pull request Apr 20, 2026
@mergify mergify Bot merged commit c028bac into sigp:unstable Apr 20, 2026
39 checks passed
@mergify mergify Bot removed the queued label Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready-for-merge This PR is ready to merge. slasher

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants