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

fix: chain: Do not update message info cache until after message validation #11202

Merged

Conversation

gammazero
Copy link
Contributor

@gammazero gammazero commented Aug 23, 2023

Related Issues

The sequence number used for replay detection was being updated before message validation confirmed that the message originated from the correct host. This would allow one host A to create a message with the ID of another host B that could then update the cached sequence number for B. While the message from A would fail validation and be ignored, the cached sequence number for B would get updated. This would lead to a temporary DoS for host B as its messages were incorrectly rejected as replays.

Host A would need to be authorized to send messages on the network, so this issue could only be caused by an authorized host.

Proposed Changes

This fixes the issue by setting the cached sequence number after message validation.

A unit test is included that demonstrates the issue is fixed. It fails with the previous code and works with the fixed code.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

The sequence number used for replay detection was being updated before message validation confirmed that the message originated from the correct host. This would allow one host A to create a message with the ID of another host B that could then update the cached sequence number for B. While the message from A would fail validation and be ignored, the cached sequence number for B would get updated. This would lead to a temporary DoS for host B as its messages were incorrectly rejected as replays.

This fixes the issue by setting the cached sequence number after message validation.
@gammazero gammazero requested a review from a team as a code owner August 23, 2023 23:56
@gammazero gammazero changed the title Fix/update cached message sequence fix: chain: Do not update message info cache until after message validation Aug 24, 2023
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants