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

contributeBonus doesn't lock expiry, unlike stake

Author Revealed upon completion

Root

stake() sets the one-way expiryLocked latch on every successful deposit (if (!expiryLocked) { expiryLocked = true; }), but contributeBonus() has no equivalent line. The latch was designed to protect anyone who has put funds into the pool from the sponsor moving the deadline out from under them — but it was wired only to the staking path, not to the bonus-contribution path, even though contributeBonus() is a permissionless fund-deposit entrypoint with the exact same reliance problem. setExpiry()'s only guard is if (expiryLocked) revert ExpiryLocked();, so as long as no one has staked yet, it's fully open regardless of how much bonus capital sits in the contract.

Impact

  • The sponsor can unilaterally change the pool's term (expiry) after third-party bonus capital is already locked in, with no on-chain signal to the contributor and no way for the contributor to object or exit (bonus contributors have no withdraw/claim path at all).

  • This shifts T = expiry in the EXPIRED-path k=2 bonus formula and reopens/shortens the staking window, changing the economics the contributor funded against.

  • Bounded impact: no funds are stolen or trapped — the bonus still eventually reaches stakers or recoveryAddress under the normal rules. The harm is loss of the term guarantee the contributor relied on, not loss of tokens.

Description

Normal behavior:
expiry is the sponsor-controlled pool deadline used to gate staking/bonus deposits and to anchor the EXPIRED-path bonus calculation. To protect anyone who has already put funds into the pool, the contract is designed so that once a deposit has been made against a given expiry, the sponsor can no longer move it — this is enforced by the one-way expiryLocked flag, which setExpiry() checks before allowing any change (if (expiryLocked) revert ExpiryLocked();).

The issue:
expiryLocked is only ever set inside stake() (if (!expiryLocked) { expiryLocked = true; }). contributeBonus() — which is also a permissionless entrypoint that locks real ERC20 value into the pool — never sets this flag. As a result, if a bonus contributor funds the pool before any staker has deposited, expiryLocked remains false, and the sponsor can still freely call setExpiry() to move the deadline, with no on-chain signal to the contributor and no lock preventing it.

function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
if (amount == 0) revert InvalidAmount();
if (amount < minStake) revert BelowMinStake();
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
if (block.timestamp >= expiry) revert StakingClosed();
_assertDepositsAllowed(_observePoolState());
​
@> if (!expiryLocked) {
@> expiryLocked = true;
@> }
​
// ... balance-diff transfer + accounting
}
​
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
​
// Balance-diff defense-in-depth — see `stake`.
uint256 balanceBefore = stakeToken.balanceOf(address(this));
stakeToken.safeTransferFrom(msg.sender, address(this), amount);
uint256 balanceAfter = stakeToken.balanceOf(address(this));
uint256 received = balanceAfter > balanceBefore ? balanceAfter - balanceBefore : 0;
if (received == 0) revert NoTokensReceived();
​
totalBonus += received;
​
emit BonusContributed(msg.sender, received);
}
​
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;
expiry = uint32(newExpiry);
​
emit ExpiryUpdated(oldExpiry, newExpiry);
}

Risk

Likelihood:

  • Occurs whenever a bonus contributor funds a pool during the normal window before the first staker deposits — a routine, expected sequence, since sponsors are encouraged to seed sponsor/community bonus contributions early to attract stakers.

Occurs on every setExpiry() call the sponsor makes after that point, since the function performs no check for existing bonus capital — the sponsor doesn't need to time anything or exploit a race, the gap is open by default until the first stake lands.

Impact:

  • The sponsor unilaterally changes the pool's staking window and the T used in the EXPIRED bonus formula after third-party capital is already committed, with no on-chain notice to the contributor.

Bonus contributors have no claim, withdraw, or exit path in the contract, so they carry this risk with zero recourse once their tokens are transferred in.

Proof of Concept

Drop it into test/unit/ — it extends the repo's own BaseConfidencePoolTest so it picks up the existing mocks automatically.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
/// @title ConfidencePool — audit findings PoC
/// @notice Drop this file into `test/unit/` in the ConfidencePool repo. It reuses the project's
/// own `BaseConfidencePoolTest` harness, so it will pick up the same mocks (MockAttackRegistry,
/// MockSafeHarborRegistry, MockAgreement, MockERC20) already wired in `setUp()`.
/// Run with: forge test --match-contract ConfidencePoolFindingsTest -vvvv
contract ConfidencePoolFindingsTest is BaseConfidencePoolTest {
/// ------------------------------------------------------------------
/// FINDING 1 (Low) — `contributeBonus` does not lock `expiry`.
///
/// `stake()` sets the one-way `expiryLocked` latch on the caller's first successful deposit.
/// `contributeBonus()` has no equivalent latch. A bonus contributor who funds a pool before
/// any staker has deposited is trusting the *published* `expiry` as the term their donation is
/// meant to reward. The sponsor can silently move that goalpost — pushing `expiry` far into
/// the future (or pulling it closer, subject only to `_MIN_EXPIRY_LEAD`) — with no on-chain
/// signal to the contributor and no recourse, since bonus contributors hold no claim rights.
///
/// Impact: sponsor can unilaterally re-negotiate the pool's term after third-party funds are
/// already locked in, changing (a) how long the staking window stays open, and (b) the `T`
/// used in the EXPIRED-path bonus formula (`outcomeFlaggedAt = expiry`), without the
/// contributor's consent and without moving `expiryLocked`.
/// ------------------------------------------------------------------
function test_Finding1_ContributeBonusDoesNotLockExpiry() public {
uint256 originalExpiry = pool.expiry();
​
// A bonus contributor funds the pool, implicitly trusting the published term.
_contributeBonus(dave, 100 * ONE);
assertEq(pool.totalBonus(), 100 * ONE);
​
// expiryLocked is still false — only `stake()` flips it.
assertFalse(
pool.expiryLocked(),
"expiryLocked should still be false after a bonus-only deposit"
);
​
// Sponsor (pool owner, `address(this)` per BaseConfidencePoolTest.setUp) unilaterally
// extends the term by ~200 days with zero on-chain friction.
uint256 newExpiry = originalExpiry + 200 days;
pool.setExpiry(newExpiry);
​
assertEq(
pool.expiry(),
newExpiry,
"sponsor was able to move expiry after bonus funding"
);
assertGt(pool.expiry(), originalExpiry);
​
// A staker can now still deposit at any point across a much longer window than the
// contributor was shown when they funded the pool, changing dilution/timing dynamics
// for the k=2 bonus split without the contributor's knowledge or consent.
}

Recommended Mitigation

This makes expiryLocked fire on the first value-locking deposit of either kind, matching the guarantee setExpiry() is supposed to enforce. No changes needed to setExpiry() itself — it already gates correctly on the flag once the flag is set at the right time.

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());
​
+ if (!expiryLocked) {
+ expiryLocked = true;
+ }
+
// Balance-diff defense-in-depth against the factory allowlist admitting a fee-on-transfer
// or rebasing token (governance error, or a proxy-token upgrade post-allowlist).
uint256 balanceBefore = stakeToken.balanceOf(address(this));
stakeToken.safeTransferFrom(msg.sender, address(this), amount);
uint256 balanceAfter = stakeToken.balanceOf(address(this));
uint256 received = balanceAfter > balanceBefore ? balanceAfter - balanceBefore : 0;
if (received == 0) revert NoTokensReceived();
​
totalBonus += received;
​
emit BonusContributed(msg.sender, received);
}

Support

FAQs

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

Give us feedback!