Skip to content

Fin sequence arithmetic can overflow in proposal-part streaming #445

Description

@memosr

StreamState::insert computes the expected message count as msg.sequence as usize + 1. The sequence field is an unvalidated u64 from the gossip wire, so a peer can send a Fin part with sequence = u64::MAX.

  • Debug/test builds: overflow panic, node crashes.
  • Release builds: wraps to 0. Currently safe only by accident, the stream stays incomplete and gets evicted.

The existing comment also claims the +1 cannot overflow, which is not accurate since the value comes from the wire, not from MAX_MESSAGES_PER_STREAM.

Proposed fix: use saturating_add(1), correct the comment, and add a regression test with sequence = u64::MAX.

This was previously proposed in #195 (closed under the new contribution policy). I would like to be assigned to this issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions