Root + Impact
Description
-
When an attacker drains the pool funds using the contracts out of the scope this agreement, the moderator may decide to flag it as SURVIVED.
-
This way, since the attacker has already drained the pool, there is no or little funds to cover the claims of stakers. This issue is not currently covered in the payment conditions. Therefore, it causes a PANIC error.
function claimSurvived() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED) revert OutcomeNotSet();
if (hasClaimed[msg.sender]) revert InvalidAmount();
uint256 userEligible = eligibleStake[msg.sender];
if (userEligible == 0) revert InvalidAmount();
_clampUserSums(msg.sender);
hasClaimed[msg.sender] = true;
uint256 bonusShare = _bonusShare(msg.sender, userEligible);
uint256 payout = userEligible + bonusShare;
totalEligibleStake -= userEligible;
claimedBonus += bonusShare;
delete eligibleStake[msg.sender];
delete userSumStakeTime[msg.sender];
delete userSumStakeTimeSq[msg.sender];
if (!claimsStarted) claimsStarted = true;
@> stakeToken.safeTransfer(msg.sender, payout);
emit ClaimSurvived(msg.sender, userEligible, bonusShare);
}
Risk
Likelihood:
Impact:
Proof of Concept
Please open a new file like ConfidencePoolContest.t.sol in the unit test folder. Then, run the test using forge test --mt test_claimSurvived_failsWhenModeratorFlagsOutOfScopeCorruptedAsSurvived -vvv.
pragma solidity 0.8.26;
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {stdStorage, StdStorage} from "forge-std/StdStorage.sol";
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";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
import {
IERC20Errors
} from "lib/openzeppelin-contracts/contracts/interfaces/draft-IERC6093.sol";
contract ConfidencePoolContestTest is BaseConfidencePoolTest {
using stdStorage for StdStorage;
event AttackerBountyClaimed(
address indexed attacker,
uint256 amount,
uint256 totalClaimed,
uint256 totalEntitlement
);
function test_claimSurvived_failsWhenModeratorFlagsOutOfScopeCorruptedAsSurvived()
external
{
_stake(alice, 5 * ONE);
_passThroughUnderAttack();
attackRegistry.setAgreementState(
IAttackRegistry.ContractState.CORRUPTED
);
deal(address(token), address(pool), 0);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
vm.prank(alice);
vm.expectPartialRevert(IERC20Errors.ERC20InsufficientBalance.selector);
pool.claimSurvived();
}
}
Recommended Mitigation
There are 2 solutions:
The easiest (but not the proper) one is to check the pool's balance and if it is not enough to cover the claim, revert with the appropriate custom error.
But the proper solution is to transfer the funds already transferred to the recoveryAddress (by the good faith attacker) back to the pool.
function claimSurvived() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED) revert OutcomeNotSet();
if (hasClaimed[msg.sender]) revert InvalidAmount();
uint256 userEligible = eligibleStake[msg.sender];
if (userEligible == 0) revert InvalidAmount();
_clampUserSums(msg.sender);
hasClaimed[msg.sender] = true;
uint256 bonusShare = _bonusShare(msg.sender, userEligible);
uint256 payout = userEligible + bonusShare;
+ uint256 freeBalance = stakeToken.balanceOf(address(this));
+ if (payout > freeBalance) revert InvalidAmount();
totalEligibleStake -= userEligible;
claimedBonus += bonusShare;
delete eligibleStake[msg.sender];
delete userSumStakeTime[msg.sender];
delete userSumStakeTimeSq[msg.sender];
if (!claimsStarted) claimsStarted = true;
stakeToken.safeTransfer(msg.sender, payout);
emit ClaimSurvived(msg.sender, userEligible, bonusShare);
}
