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

Temporary pool underfunding locks good-faith attacker out of claiming remainder of bounty entitlement after deadline

Author Revealed upon completion

Root + Impact

Description

The goodFaith attacker should be able to claim their entire bounty amount as long as they actively call claimAttackerBountyBut that is not the case if the stakeToken used in the pool is a debasing token.

When a pool contains a debasing token that decreases the contract balance below the snapshotted bountyEntitlement, the attacker's initial claim is capped by the current balance of the pool, leaving bountyClaimed < bountyEntitlement. once corruptedClaimDeadline passes, the contract permanently blocks the attacker from claiming the remaining balance by reverting with ClaimWindowExpired(), even if the pool balance later replenished (e.g via direct deposit or depositBonus function call). These remaining funds are instead swept to the recoveryAddress via sweepUnclaimedCorrupted.


Let's walk through your possible arguments and my response

Your argument: The protocol does not accept weird tokens, such as rebase tokens and others

My response: Yes you have a point but while the protocol stated that they don't allow weird tokens they explicitly left a comment inside the stake function that explains why they defensively program against such tokens despite the fact that they don't allow such tokens they still write the ConfidencePool.sol codebase to be bulletProof against such tokens because of their reasons as stated inside the stake function of the contract. shown below

function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
...
// 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).
// aderyn-fp-next-line(reentrancy-state-change)
// get the stake token balance of this contract
uint256 balanceBefore = stakeToken.balanceOf(address(this));
// transfer from the msg.sender to this account the stake amount
stakeToken.safeTransferFrom(msg.sender, address(this), amount);
// aderyn-fp-next-line(reentrancy-state-change)
// get stake token balance of this contract after the user deposited
uint256 balanceAfter = stakeToken.balanceOf(address(this));
// compute the stake token amount received
uint256 received = balanceAfter > balanceBefore
? balanceAfter - balanceBefore
: 0;
...
}

As you can see, the programmer explicitly made it clear that they programmed the contract to be bullet-proofed against weird tokens in the case that their is a governace error that allowed such tokens or a proxy-token that was not initally weird when it was approved becomes weird later after upgrade of it's implementation contract.

So that makes your statement The protocol does not accept weird tokens, such as rebase tokens and others invalid and totally wrong.

// Root cause in the codebase with @> marks to highlight the relevant section
function claimAttackerBounty() external nonReentrant {
...
// ensure claim window haven't pass
@> if (block.timestamp > corruptedClaimDeadline)
@> revert ClaimWindowExpired();
630: // compute what is left to pay whitehat
631: uint256 remaining = bountyEntitlement - bountyClaimed;
...
}

The claimAttackerBounty function reverts as shown above from the point corruptedClaimDeadline pass which makes sense as that was how it was designed but the attacker is actively claiming and looking forward to claim the balance of the bounty amount as soon as the pool balance gets top-up but they couldn't because no top-up came until corruptedClaimDeadline passed and now the protocol punishes them, it is not their fault that no top-up arrived till after corruptedClaimDeadline


You may argue that yes that is completely fine there is nothing the protocol can do about it the claiming time has passed and that is it, the attacker should forget their balance which doesn't make sense but let's assume for a second that we accept that what about the state of the pool bountyClaimed and bountyEntitlement That isn't the same as when the attacker has claimed, and the corruptedClaimDeadline has passed?

Thats an inconsistency in the protocol state which is not supposed to happen, it is either the protocol open claim for the attacker till they are able to claim all their bountyEntitlement or they set bountyClaimed to bountyEntitlement once the attacker claim to signify that the attacker have successfully claimed their bounty regardles of wether the token balance debased or not


Risk

Likelihood:

This will happen when the attacker claim their bounty while the total balance of the pool is lesser than the snapShotted amount bountyEntitlement at the moment of resolution

  • Reason 2

Impact:

It will make the attacker lose out on some of their bountyEntitlement not because they are inactive, but because the token debase and no top-up happened till corruptedClaimDeadline passed. Meaning the protocol punished them for what is out of their control

  • Impact 2

Proof of Concept

Create a file named ConfidenceBountyDebaseVulnerability.t.test and paste the code below in it. then run the test with forge test --mt testBountyClaimedBlockDeadlineWhenDebased

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {ConfidencePool} from "src/ConfidencePool.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract BountyDebaseVulnerabilityTest is BaseConfidencePoolTest {
function testBountyClaimBlockedByDeadlineWhenDebased() external {
// 1. Stake and contribute bonus normally (total entitlement = 150 * ONE)
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
​
// 2. Transition the registry through UNDER_ATTACK
_passThroughUnderAttack();
​
// 3. Agreement gets corrupted, moderator flags the outcome as CORRUPTED in good faith
attackRegistry.setAgreementState(
IAttackRegistry.ContractState.CORRUPTED
);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
​
// bountyEntitlement = 150 * ONE
assertEq(pool.bountyEntitlement(), 150 * ONE);
​
// 4. Simulate a debasing token: remove 30 * ONE from the pool balance directly
// The contract balance drops to 120 * ONE
uint256 currentBalance = token.balanceOf(address(pool));
assertEq(currentBalance, 150 * ONE);
vm.prank(address(pool));
token.transfer(address(0xdead), 30 * ONE);
assertEq(token.balanceOf(address(pool)), 120 * ONE);
​
// 5. Attacker calls claimAttackerBounty()
// Attacker wants to claim 150, but can only claim 120 due to debase (freeBalance = 120)
uint256 attackerBefore = token.balanceOf(attacker);
vm.prank(attacker);
pool.claimAttackerBounty();
​
// Attacker only gets 120 * ONE, and bountyClaimed becomes 120 * ONE
assertEq(token.balanceOf(attacker) - attackerBefore, 120 * ONE);
assertEq(pool.bountyClaimed(), 120 * ONE);
assertEq(token.balanceOf(address(pool)), 0);
​
// 6. Time passes beyond the corrupted claim deadline
uint256 deadline = pool.corruptedClaimDeadline();
vm.warp(deadline + 1);
​
// 7. Token balance is restored (e.g. via direct donation or a positive rebase of 30 * ONE)
token.mint(address(pool), 30 * ONE);
assertEq(token.balanceOf(address(pool)), 30 * ONE);
​
// 8. Attacker tries to claim the remaining 30 * ONE, but it reverts because the deadline has passed
vm.prank(attacker);
vm.expectRevert(IConfidencePool.ClaimWindowExpired.selector);
pool.claimAttackerBounty();
​
// 9. Anyone can now sweep the remaining 30 * ONE to recoveryAddress, leaving the attacker permanently short-changed
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedCorrupted();
assertEq(token.balanceOf(recovery) - recoveryBefore, 30 * ONE);
assertEq(token.balanceOf(address(pool)), 0);
}
}
​

Recommended Mitigation



You can either set bountyClaimed to be equal to bountyEntitlement once the attacker has claimed, regardless of whether the amount available in the pool is equal bountyEntitlement or not, which ensures that the attacker is not owed while declining his payment

- remove this code
bountyClaimed = newBountyClaimed;
+ add this code
bountyClaimed = bountyEntitlement;

Support

FAQs

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

Give us feedback!