ava-labs / ava-labs/Simplex

Stale CachedStorage entries shadow finalized blocks, desyncing epoch-transition consensus checks

Open
#549 0 comments 0 reactions 0 assignees View on GitHub
medium security
Dominant language
Go
Stars
22
Forks
4
Avg merge
2d 14h
Merged PRs (30d)
34

Description

## Details
CachedStorage.Retrieve(seq, digest) (adapters.go:118-147) is the block retriever wired into the metadata state machine (instance.go:462). It consults the in-memory cache of verified-but-unfinalized blocks before persistent storage and returns every cache hit with a nil finalization; for an empty digest it returns the first cache entry whose seq matches under randomized map iteration. Consensus-critical epoch-transition checks (areWeReadyToTransitionEpoch, verifyNextPChainRefHeightNormal, buildBlockOrTransitionEpoch, computeSimplexEpochInfoForSealingBlock, WaitForPendingBlock) look up the sealing block with an empty digest and interpret nil finalization as 'sealing block not finalized', so any cache entry at the sealing block's seq shadows the finalized block in storage and flips these decisions.

A stale entry can be planted because cachedBlock.Verify (adapters.go:87-93) inserts every successfully verified block unconditionally, verification runs asynchronously on a sequential scheduler, and finalizations are indexed concurrently on the message-handling path (persistFinalization -> indexFinalizations -> Storage.Index). RemoveOldTasks discards only dependency-pending tasks - a task already queued or running is never cancelled - and CachedStorage.Index prunes the cache only at indexing time. Thus when a proposal B at seq s from round r is still being verified while a different block B' at the same seq is finalized and indexed (guaranteed on the next round's leader, which never scheduler-verifies its own proposal), B's late insert lands after the prune and persists until the next Index call. The affected validator then wrongly treats the sealing block as unfinalized: it rejects the new epoch's valid first block (voting empty via triggerEmptyBlockNotarization) and, as leader, builds Telocks whose metadata fails verifyEpochNumber on all peers, degrading liveness until the node indexes the next externally finalized block via replication (OnlyVMVerifyOpt bypasses the MSM, so healing is not blocked) or restarts. The stale entry survives the epoch restart because the Instance reuses the same CachedStorage across epochs.

Two secondary defects in the same function: (1) on the storage path, block.Digest() is evaluated before err is checked and any storage error under a non-empty digest is rewritten as ErrBlockNotFound, discarding the failure cause (currently without distinct consequence: the only consumer, the MSM, treats all retrieval errors alike, and the engine's retrieveBlockOrHalt uses the separate EpochAwareStorage.Retrieve(seq) path that bypasses this function); (2) the empty-digest map scan is nondeterministic between same-seq forks - with the NoopAuxiliaryInfoApp wired in this repository the only content-sensitive consumer (collectAuxiliaryInfo) is unreachable, so that is a latent hazard for deployments supplying a real AuxiliaryInfoApp rather than an active vulnerability here.

## Evidence
1. [adapters.go:118–147](https://github.com/ava-labs/Simplex/blob/main/adapters.go#L118-L147)
Primary defect: Retrieve consults the unfinalized-block cache before persistent storage and returns every cache hit with a nil finalization (lines 121-134), so any cache entry at a seq shadows a finalized block in storage; for an empty digest the map scan returns an arbitrary same-seq entry (lines 127-134). On the storage path, block.Digest() is evaluated before err is checked and real storage errors under a non-empty digest are rewritten as ErrBlockNotFound (lines 138-141).
2. [adapters.go:87–93](https://github.com/ava-labs/Simplex/blob/main/adapters.go#L87-L93)
cachedBlock.Verify inserts every successfully verified block into the cache unconditionally, with no check against already-indexed seqs. Verification runs asynchronously, so a task completing after its seq was finalized and indexed plants a stale entry that CachedStorage.Index's pruning (which ran earlier, at indexing time) never removes.
3. [msm/msm.go:1363–1381](https://github.com/ava-labs/Simplex/blob/main/msm/msm.go#L1363-L1381)
areWeReadyToTransitionEpoch looks up the sealing block with GetBlock(sealingBlockSeq, [32]byte{}) and interprets finalization == nil as 'not finalized'. Because the cache path always returns nil finalization and is consulted before storage, a stale cached block at the sealing seq makes buildBlockEpochSealed build Telocks and verifyBlockEpochSealed reject the new epoch's valid first block even though storage holds the sealing block finalized.
4. [msm/msm.go:670–679](https://github.com/ava-labs/Simplex/blob/main/msm/msm.go#L670-L679)
verifyNextPChainRefHeightNormal performs the same empty-digest sealing-block lookup and rejects transition-initiating proposals with errPrevSealingBlockNotFinalized when it receives a nil finalization; buildBlockOrTransitionEpoch (lines 530-538) and computeSimplexEpochInfoForSealingBlock (lines 1324-1332) share the pattern, so a stale cache entry flips both build-side and verify-side epoch-transition decisions.
5. [simplex/epoch.go:1285–1301](https://github.com/ava-labs/Simplex/blob/main/simplex/epoch.go#L1285-L1301)
persistFinalization runs on the message-handling path (also reached by the round leader assembling finalize votes for its own never-scheduler-verified proposal) and calls indexFinalizations -> Storage.Index, pruning the cache, concurrently with block-verification tasks. RemoveOldTasks (line 1293) is the only staleness protection and covers only dependency-pending tasks.
6. [common/block\_scheduler.go:180–194](https://github.com/ava-labs/Simplex/blob/main/common/block_scheduler.go#L180-L194)
RemoveOldTasks removes only tasks still waiting in the dependency list; tasks already handed to the sequential scheduler (queued or executing block.Verify) cannot be cancelled, leaving the window in which a verification finishing after Index(seq) inserts a stale same-seq entry into the cache.
7. [msm/msm.go:1247–1256](https://github.com/ava-labs/Simplex/blob/main/msm/msm.go#L1247-L1256)
Latent hazard only: collectAuxiliaryInfo walks PrevAuxInfoSeq links with getBlock(seq, [32]byte{}), so under a non-noop AuxiliaryInfoApp two cached same-seq forks with different aux Info would yield nondeterministic histories and divergent auxInfoDigest values. With the NoopAuxiliaryInfoApp wired in instance.go:460, no block can legally carry non-empty Info or a non-zero PrevAuxInfoSeq, so this loop body is unreachable in this repository's deployment.

## Impact
The victim validator's consensus behavior is altered in an attacker-directed way: it votes empty against valid new-epoch proposals and produces invalid Telock proposals while leader, so its consensus participation is corrupted (LOW integrity) and liveness of rounds it leads is lost (LOW availability). The effect is transient - it heals once the node indexes the next finalized block via replication or restarts - and each race occurrence affects a single node; no finalized data is corrupted and nothing is disclosed.

## Reproduction steps
1. A malicious validator, as leader of round r during the epoch-sealing phase, sends a valid proposal B at sealing seq s late in the round with a slow-to-verify inner block. Round r empty-notarizes; the next leader N builds and finalizes its own B' at seq s (own proposals are never scheduler-verified). N indexes B' on the message path, pruning the cache, while B is still verifying; B's success then inserts a stale seq-s entry. N's sealing-block lookups return nil finalization, so N rejects the new epoch's valid first block and proposes invalid Telocks until replication heals it. Requires validator role and winning the timing race during an epoch transition.

## Recommended fix
1. CachedStorage.Retrieve consults the unfinalized-block cache before persistent storage and reports every cache hit with a nil finalization, so a cache entry at an already-finalized seq shadows the finalized block and flips 'is the sealing block finalized' consensus decisions. Fix criteria: A seq that is finalized in storage must always be reported with its finalization regardless of cache contents. Verify that areWeReadyToTransitionEpoch and verifyNextPChainRefHeightNormal return the finalized answer when the sealing block is in storage while a same-seq block sits in the cache.
2. cachedBlock.Verify inserts every successfully verified block into the cache without checking whether the block's seq has already been indexed, and in-flight verification tasks are not cancelled on finalization, so a verification completing after CachedStorage.Index pruned that seq plants a stale entry that persists across epoch restarts. Fix criteria: A verification that completes after its seq is finalized must not leave an entry in the cache (synchronize insertion with indexing, or validate insertions/lookups against the storage height). Verify that a verification task finishing after Index(seq) cannot cause a subsequent Retrieve(seq, empty digest) to return a cached block.
3. In Retrieve's storage path, block.Digest() is compared before the GetBlock error is checked, and any storage error under a non-empty digest is rewritten as ErrBlockNotFound, discarding the failure cause. Fix criteria: Storage errors must propagate to callers distinctly from 'not found', and the digest comparison must run only on a successfully retrieved block. Verify that an injected GetBlock I/O error surfaces to the MSM as that error rather than ErrBlockNotFound.
4. Empty-digest lookups resolve same-seq collisions by randomized map iteration, returning an arbitrary fork; a content-sensitive consumer (collectAuxiliaryInfo under a non-noop AuxiliaryInfoApp) would derive divergent aux-info histories and digests across nodes. Fix criteria: Empty-digest lookups must be deterministic and identical across nodes for the same cache contents (canonical resolution or a total order among same-seq candidates). Verify that with two cached forks at one seq, repeated lookups return the same block on every node.

---
**Severity:** MEDIUM
**Status:** Open
**Category:** Race condition
**CWE:** [CWE-362](https://cwe.mitre.org/data/definitions/362.html)
**Repository:** ava-labs/Simplex
**Branch:** main
**Date created:** 2026-08-21

---

---
_Related existing issue: #501_

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with CachedStorage.Retrieve and cachedBlock.Verify in adapters.go, then trace the affected sealing-block lookups in msm/msm.go and indexing in simplex/epoch.go. Verify the race and storage-error behavior against the listed fix criteria: finalized storage entries must win, late verification must not reinsert stale entries, storage errors must propagate, and empty-digest lookups must be deterministic.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
distributed-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.