**Audit seed **
JBDistributor._takeSnapshotOf, collectVestedRewards(), beginVesting(), roundSnapshotBlock, SnapshotCreated
Repos involved
nana-distributor-v6 (JBDistributor and its JBTokenDistributor / JB721Distributor callers)
Root cause — the fundamental issue, not the symptom
_takeSnapshotOf() uses snapshot.balance != 0 as the only proof that a snapshot already exists for (hook, token, round). If the first call in a round observes a zero balance, the helper stores a zero-valued snapshot and later treats that same entry as if it were never initialized. A subsequent call in the same round then overwrites the stored snapshot with the new balance. That means “snapshot taken” is inferred from a mutable business value instead of an explicit initialization flag.
Impact — what an attacker gains, with concrete values
A caller can force an empty snapshot early in a round, then let rewards arrive later in the same round, and then trigger a second call that re-snapshots the newly funded balance. The practical result is that tokens deposited after the first call can still be included in the current round instead of waiting for the next round. For a deposit of X reward tokens, that lets the attacker or their chosen claim path pull X into the current vesting cycle and collect their share immediately, rather than having the deposit remain excluded until the next round boundary. This is a real timing/misallocation effect, not a theoretical bookkeeping issue, because both beginVesting() and collectVestedRewards() rely on _takeSnapshotOf() to decide the distributable amount.
Proof of concept — step-by-step exploitation sequence with function calls
- A new round begins and the distributor’s tracked balance for
hook/token is still 0.
- The attacker calls
collectVestedRewards(hook, [theirTokenId], [rewardToken], attacker). This reaches _takeSnapshotOf() and stores {balance: 0, vestingAmount: 0} for the current round. The call does not need any distributable balance to persist that zero snapshot.
- Later in the same round, a terminal/controller funds the distributor through
processSplitWith(...), or anyone funds it through fund(...), increasing _balanceOf[hook][rewardToken].
- The attacker calls
collectVestedRewards(...) again in the same round. _takeSnapshotOf() reads the stored zero snapshot, treats it as unset because snapshot.balance != 0 is false, and overwrites it with the new non-zero balance.
- The newly funded tokens are now vested/collected in the current round using the already-anchored round snapshot block, rather than being held out until the next round.
Why this survived self-review — the strongest counter-argument and why it failed
The strongest defense is that a zero snapshot might look harmless because “there was nothing to distribute at that moment.” That misses the invariant break: the helper is supposed to record a once-per-round snapshot, but zero is being used as a sentinel for “not initialized.” The code then allows a later call to replace the first snapshot, which means the round is no longer defined by its first snapshot at all. The result is not just an empty round; it is a mutable round boundary that can absorb later deposits.
Recommended fix
Store an explicit initialization flag per (hook, token, round) and never use balance == 0 as the “snapshot exists” check. The snapshot should be write-once for the round, even when the balance is zero. A zero balance should remain a valid snapshot value, not a signal to resnapshot later.
**Audit seed **
JBDistributor._takeSnapshotOf,collectVestedRewards(),beginVesting(),roundSnapshotBlock,SnapshotCreatedRepos involved
nana-distributor-v6(JBDistributorand itsJBTokenDistributor/JB721Distributorcallers)Root cause — the fundamental issue, not the symptom
_takeSnapshotOf()usessnapshot.balance != 0as the only proof that a snapshot already exists for(hook, token, round). If the first call in a round observes a zero balance, the helper stores a zero-valued snapshot and later treats that same entry as if it were never initialized. A subsequent call in the same round then overwrites the stored snapshot with the new balance. That means “snapshot taken” is inferred from a mutable business value instead of an explicit initialization flag.Impact — what an attacker gains, with concrete values
A caller can force an empty snapshot early in a round, then let rewards arrive later in the same round, and then trigger a second call that re-snapshots the newly funded balance. The practical result is that tokens deposited after the first call can still be included in the current round instead of waiting for the next round. For a deposit of
Xreward tokens, that lets the attacker or their chosen claim path pullXinto the current vesting cycle and collect their share immediately, rather than having the deposit remain excluded until the next round boundary. This is a real timing/misallocation effect, not a theoretical bookkeeping issue, because bothbeginVesting()andcollectVestedRewards()rely on_takeSnapshotOf()to decide the distributable amount.Proof of concept — step-by-step exploitation sequence with function calls
hook/tokenis still0.collectVestedRewards(hook, [theirTokenId], [rewardToken], attacker). This reaches_takeSnapshotOf()and stores{balance: 0, vestingAmount: 0}for the current round. The call does not need any distributable balance to persist that zero snapshot.processSplitWith(...), or anyone funds it throughfund(...), increasing_balanceOf[hook][rewardToken].collectVestedRewards(...)again in the same round._takeSnapshotOf()reads the stored zero snapshot, treats it as unset becausesnapshot.balance != 0is false, and overwrites it with the new non-zero balance.Why this survived self-review — the strongest counter-argument and why it failed
The strongest defense is that a zero snapshot might look harmless because “there was nothing to distribute at that moment.” That misses the invariant break: the helper is supposed to record a once-per-round snapshot, but zero is being used as a sentinel for “not initialized.” The code then allows a later call to replace the first snapshot, which means the round is no longer defined by its first snapshot at all. The result is not just an empty round; it is a mutable round boundary that can absorb later deposits.
Recommended fix
Store an explicit initialization flag per
(hook, token, round)and never usebalance == 0as the “snapshot exists” check. The snapshot should be write-once for the round, even when the balance is zero. A zero balance should remain a valid snapshot value, not a signal to resnapshot later.