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

[L-2] `contributeBonus()` fails to set `expiryLocked`, thus allowing the sponsor to alter `expiry` after the bonus funds have been staked(contributed).

Author Revealed upon completion

Description:

From the codebase, the expiryLocked is the pool's one way commitment latch, once it is set. the setExpiry is permanently disabled.

function setExpiry(uint256 newExpiry) external onlyOwner {
@> if (expiryLocked) revert ExpiryLocked();
if (newExpiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (newExpiry > type(uint32).max) revert ExpiryTooFar();
​
uint256 oldExpiry = expiry;
// forge-lint: disable-next-line(unsafe-typecast)
expiry = uint32(newExpiry);
​
emit ExpiryUpdated(oldExpiry, newExpiry);
}

Per the actor table, the sponsors authority over expiry is explicitly scoped:

cannot alter expiry after the first stake.

In practice, the latch is wired to only of the two deposit-shaped entry points.

function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
...
if (!expiryLocked) {
expiryLocked = true;
}
...
}

and then we have the contributeBonus function:

function contributeBonus(uint256 amount) external nonReentrant whenPoolNotPaused {
if (amount == 0) revert InvalidAmount();
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
if (block.timestamp >= expiry) revert StakingClosed();
_assertDepositsAllowed(_observePoolState());
// no expiryLocked write anywhere in this function
...
totalBonus += received;
emit BonusContributed(msg.sender, received);
}

From Observation, the contributeBonus() function shares every other structural property of the stake() function i.e:

The same UNRESOLVED/pre-expiry/registry-gating checks.
The same balance-diff based safeTransferFrom pull.
Lastly, the same permissionless capability.

But it never touches/updates the expiryLocked flag.

Capital commitment via the contributeBonus() function is therefore not recognized by the latch as a "first commitment" event, even though it is real, irreversible capital sitting in the pool from the moment it lands.

Impact:

This is more of a direct deviation from the documentated limitation:

"Cannot alter expiry after the first stake"

Yes, Bonus contributors have no claim rights on the funds, so this isn't a direct principal-theft path but this is more of a real, sponsor-triggerable divergence between actual contract behaviour.

Proof of Concept:

Would recommend placing this in the ConfidencePool.t.sol file.

function test_contributeBonusDoesNotLockExpiry() public {
// 1. Pool created with expiry = T0 (via factory / initialize)
uint256 originalExpiry = pool.expiry();
​
// 2. Bonus contributor(carol) deposits BEFORE any staker
_contributeBonus(carol, 40 * ONE);
​
// 3. expiryLocked is still false — sponsor can still move the deadline
assertEq(pool.expiryLocked(), false);
​
// 4. Sponsor moves expiry after bonus capital is already committed
pool.setExpiry(originalExpiry + 30 days); // succeeds — should not be possible
assertEq(pool.expiry(), originalExpiry + 30 days);
​
// Compare: had a staker gone first instead of the bonus contributor,
// this same setExpiry call would revert with ExpiryLocked().
}

Recommended Mitigation:

Set the latch in contributeBonus function as well, mirroring stake()

function contributeBonus(uint256 amount) external nonReentrant whenPoolNotPaused {
...
+ if (!expiryLocked) {
+ expiryLocked = true;
+ }
...
}
​

Support

FAQs

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

Give us feedback!