Skip to content

[Security][tentative] Medium (correctness): Syncer SetFloor Stale Round Fetches Below Floor #357

Description

@evonide

Context

SetFloor advances last_processed_height but leaves last_processed_round unchanged. New Subscribe calls after the floor can therefore request notarized data for a round that is not older than the stale round marker, and the notarized delivery path caches and reports the block without checking that the delivered block is above the height floor.

Claim

A local component or catch-up path raises the sync floor, then a later subscription includes a stale round whose block height is at or below the new floor but whose round is still not less than last_processed_round. The resolver fetches a notarized response and syncer delivers it as fresh notarized activity. SetFloor advances last_processed_height but leaves last_processed_round unchanged. New Subscribe calls after the floor can therefore request notarized data for a round that is not older than the stale round marker, and the notarized delivery path caches and reports the block without checking that the delivered block is above the height floor.

Flow

The path is reachable when explicit SetFloor advances the height floor without advancing last_processed_round. A live notarized Summit block can remain accepted after the floor jump, allowing below-floor notarized data to be cached.

Impact

Below-floor notarized blocks can repopulate cache, notify subscribers, and report notarized-block updates after the node intended to prune that lifecycle range. This can keep obsolete resolver work and application notifications alive during recovery or catch-up.

Root Cause

The floor boundary is split across height and round state: update_processed_height only updates height and only retains finalized-height requests, while the subscription gate uses the stale round marker.

Code

  • Subscriptions only reject rounds older than last_processed_round:

    summit/syncer/src/actor.rs

    Lines 670 to 702 in ed2c5c8

    Message::Subscribe { round, commitment, response } => {
    // Check for block locally
    if let Some(block) = self.find_block(&mut buffer, commitment).await {
    response.send_lossy(block);
    continue;
    }
    // We don't have the block locally, so fetch the block from the network
    // if we have an associated view. If we only have the digest, don't make
    // the request as we wouldn't know when to drop it, and the request may
    // never complete if the block is not finalized.
    if let Some(round) = round {
    if round < self.last_processed_round {
    warn!(
    ?round,
    ?commitment,
    last_processed_round = ?self.last_processed_round,
    last_processed_height = %self.last_processed_height,
    tip = %self.tip,
    "subscription for block in past round that wasn't finalized - possible notarize-nullify race"
    );
    #[cfg(feature = "prom")]
    counter!("syncer_stuck_subscription_total").increment(1);
    continue;
    }
    // Attempt to fetch the block (with notarization) from the resolver.
    // If this is a valid view, this request should be fine to keep open
    // until resolution or pruning (even if the oneshot is canceled).
    debug!(?round, ?commitment, "requested block missing");
    resolver.fetch(Request::<B::Digest>::Notarized { round }).await;
    }
  • Notarized delivery gets a scheme by round and proceeds to decode without a floor-height check:

    summit/syncer/src/actor.rs

    Lines 965 to 980 in ed2c5c8

    Request::Notarized { round } => {
    let Some(scheme) = self.get_scheme_certificate_verifier(round.epoch()) else {
    debug!(
    ?round,
    floor = %self.last_processed_height,
    "ignoring stale delivery"
    );
    response.send_lossy(true);
    return false;
    };
    let Ok((notarization, block)) =
    <(Notarization<P::Scheme, B::Digest>, B)>::decode_cfg(
    value,
    &(
    scheme.certificate_codec_config(),
  • Accepted notarized deliveries are cached, reported to application, and used to notify subscribers:

    summit/syncer/src/actor.rs

    Lines 1128 to 1136 in ed2c5c8

    // Cache the notarization and block.
    self.cache_block(round, commitment, block.clone()).await;
    self.cache
    .put_notarization(round, commitment, notarization)
    .await;
    application
    .report(Update::NotarizedBlock(block.clone()))
    .await;
    self.notify_subscribers(commitment, &block).await;
  • update_processed_height advances last_processed_height and retains finalized requests, but does not update last_processed_round:

    summit/syncer/src/actor.rs

    Lines 1549 to 1570 in ed2c5c8

    /// Buffers a processed height update in memory and metrics. Does NOT sync
    /// to durable storage. Sync metadata after buffered updates to make them durable.
    async fn update_processed_height(
    &mut self,
    height: Height,
    resolver: &mut impl Resolver<Key = Request<B::Digest>>,
    ) {
    self.application_metadata.put(LATEST_KEY.clone(), height);
    self.last_processed_height = height;
    #[cfg(feature = "prom")]
    self.processed_height
    .set(self.last_processed_height.get() as i64);
    // Cancel any existing requests below the new floor.
    resolver
    .retain(
    Request::<B::Digest>::Finalized {
    height: height.get(),
    }
    .predicate(),
    )
    .await;

Related Issues/PRs

The following existing items touch the same trust boundary, adjacent root cause, or likely remediation stack.

Note that #357 is stale round state after height floor advancement.

Fix

Update both processed height and processed round when applying a floor, and reject or discard notarized deliveries whose block height is at or below last_processed_height. Add a post-SetFloor subscription test.

Metadata

Metadata

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions