Root + Impact
closePot sends tokens to every claimant in one loop. If the transfer to a single claimant reverts (for example, the token blocks that address after it has claimed), the whole closePot call reverts. The manager's cut and the payments to all other claimants are then blocked for as long as that address stays blocked, and the claimants list cannot be changed.
Description
function closePot() external onlyOwner {
...
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
All payments happen in one transaction. A revert on one transfer undoes all of them, including the manager's cut. There is no way to remove a claimant from claimants or to skip a failing payment.
Some widely used ERC-20 tokens revert when the sender or the receiver is on a block list. Some ERC-20 tokens include a blocklist and revert transfers to blocked addresses. This scenario depends on the protocol supporting such tokens.
Risk
Likelihood: Low
The token must support blocking recipients, and a claimant must become blocked after successfully claiming their initial reward.
Impact: Medium
A single blocked claimant prevents the manager and other claimants from receiving their closing payments. Closing remains blocked while the token continues rejecting that transfer.
Proof of Concept
Add the following inside the existing TestMyCut contract in test/TestMyCut.t.sol. It uses the existing setup, imports and the mintAndApproveTokens modifier.
address player3 = makeAddr("player3");
function _createFundedPot(
uint256 r1,
uint256 r2,
uint256 r3,
uint256 total
) internal returns (Pot) {
address[] memory p = new address[](3);
p[0] = player1;
p[1] = player2;
p[2] = player3;
uint256[] memory r = new uint256[](3);
r[0] = r1;
r[1] = r2;
r[2] = r3;
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
p,
r,
IERC20(ERC20Mock(weth)),
total
);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
return Pot(contest);
}
function test_PoC_blockedClaimant_blocksClosePot() public mintAndApproveTokens {
Pot pot = _createFundedPot(10, 20, 30, 60);
vm.prank(player1);
pot.claimCut();
vm.prank(player2);
pot.claimCut();
vm.warp(block.timestamp + 91 days);
bytes memory blockedError =
abi.encodeWithSignature("Error(string)", "BLOCKED");
vm.mockCallRevert(
address(weth),
abi.encodeWithSelector(
IERC20.transfer.selector,
player2,
uint256(9)
),
blockedError
);
vm.prank(user);
vm.expectRevert(blockedError);
ContestManager(conMan).closeContest(address(pot));
console.log("Player 1 balance:", weth.balanceOf(player1));
console.log("Player 2 balance:", weth.balanceOf(player2));
console.log("Pot balance:", weth.balanceOf(address(pot)));
console.log("ContestManager balance:", weth.balanceOf(conMan));
assertEq(weth.balanceOf(player1), 10);
assertEq(weth.balanceOf(player2), 20);
assertEq(weth.balanceOf(address(pot)), 30);
assertEq(weth.balanceOf(conMan), 0);
assertEq(pot.getRemainingRewards(), 30);
vm.clearMockedCalls();
vm.prank(user);
ContestManager(conMan).closeContest(address(pot));
assertEq(weth.balanceOf(player1), 19);
assertEq(weth.balanceOf(player2), 29);
assertEq(weth.balanceOf(conMan), 3);
assertEq(weth.balanceOf(address(pot)), 9);
}
Run from the repository root:
forge test --mt test_PoC_blockedClaimant_blocksClosePot -vv
Output:
[PASS] test_PoC_blockedClaimant_blocksClosePot() (gas: 1293249)
Logs:
...
Player 1 balance: 10
Player 2 balance: 20
Pot balance: 30
ContestManager balance: 0
The mock demonstrates the effect of a reverting transfer. It does not establish that blocklist-enabled tokens are supported by the contest.
Recommended Mitigation
Store closing payments instead of transferring them inside the loop. Let each claimant withdraw separately, so one failed transfer does not block everyone else.
+ import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
contract Pot is Ownable(msg.sender) {
+ using SafeERC20 for IERC20;
+
+ mapping(address => uint256) public closingPayments;
function closePot() external onlyOwner {
...
for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], claimantCut);
+ closingPayments[claimants[i]] += claimantCut;
}
}
+ function claimClosingPayment() external {
+ uint256 payment = closingPayments[msg.sender];
+ require(payment > 0, "No closing payment");
+
+ closingPayments[msg.sender] = 0;
+ i_token.safeTransfer(msg.sender, payment);
+ }
}
If a withdrawal reverts, its state changes are rolled back, allowing that claimant to retry later. Other claimants can still withdraw their payments.
