Root + Impact
Description
Normal Behavior: When a user calls cancelParticipation(), the function should fully reverse all state changes made during joinEvent(), including decrementing numberOfParticipants, reducing totalParticipantShares, and removing the user from the usersAddress array.
function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
uint256 shares = balanceOf(msg.sender);
@> _burn(msg.sender, shares);
@> IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
Risk
Likelihood: HIGH
Users naturally cancel participation before events start when circumstances change (can't participate, changed mind about team selection, etc.)
The function is explicitly designed for cancellation and users are expected to use it
Impact: HIGH
Accounting Corruption: numberOfParticipants and totalParticipantShares remain inflated, misrepresenting actual active participants
DOS Vulnerability Amplification: Cancelled users remain in usersAddress array, contributing to the unbounded loop gas consumption in _getWinnerShares() even though they're no longer participating.
Winner Calculation Errors: The _getWinnerShares() function iterates through all addresses in usersAddress including cancelled users, wasting gas and potentially causing incorrect winner share calculations
Protocol State Inconsistency: The protocol believes there are more participants than actually exist, breaking core invariants
Proof of Concept
You may copy and paste the POC on the existing test suite:
function test_stateDoesNotGetUpdated() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
address bobby = makeAddr("bobby");
mockToken.mint((bobby), 50 ether);
vm.startPrank(bobby);
mockToken.approve(address(briVault), type(uint256).max);
uint256 bobbyShares = briVault.deposit(50 ether, bobby);
console.log("user Bobby calls (deposit):");
console.log();
console.log("shares bobby has: ", bobbyShares);
console.log();
uint256 numberOfParticipantsBefore = briVault.numberOfParticipants();
console.log("user Bobby calls (joinEvent):");
console.log();
console.log("Total amount of users in the event before bobby joins: ", numberOfParticipantsBefore);
console.log();
console.log("bobby calls *joinEvent*");
console.log();
briVault.joinEvent(10);
string memory bobbysTeam = briVault.userToCountry(bobby);
console.log("Team bobby supports: ", bobbysTeam);
uint256 numberOfParticipantsAfter = briVault.numberOfParticipants();
console.log("Total amount of users in the event after bobby joined: ", numberOfParticipantsAfter);
uint256 userAddressArray = briVault._getUserAddressesLength();
console.log("Address added onto the array: ", userAddressArray);
console.log();
console.log("user bobby calls (cancelParticipation):");
console.log();
uint256 stakedAmountBefore = briVault.stakedAsset(bobby);
console.log("bobby's assets before cancellation: ", stakedAmountBefore);
briVault.cancelParticipation();
uint256 stakedAmountAfter = briVault.stakedAsset(bobby);
console.log("bobby's assets after cancellation:", stakedAmountAfter);
uint256 updatedNumberOfParticipants = briVault.numberOfParticipants();
console.log();
console.log("THE BUGS: ");
console.log();
console.log("The updated number of participations after user cancelled: (BUG)", updatedNumberOfParticipants);
assertEq(updatedNumberOfParticipants, numberOfParticipantsAfter, "The number of participants cannot be updated");
uint256 userAddressArrayAfterCancel = briVault._getUserAddressesLength();
console.log("Addresses exisitng in the array after user bobby cancelsParticipation: (BUG)", userAddressArrayAfterCancel);
assertTrue(userAddressArrayAfterCancel == 1, "the userAddress array is not updated");
}
Output:
[PASS] test_stateDoesNotGetUpdated() (gas: 1635922)
Logs:
user Bobby calls (deposit):
shares bobby has: 49250000000000000000
user Bobby calls (joinEvent):
Total amount of users in the event before bobby joins: 0
bobby calls *joinEvent*
Team bobby supports: Japan
Total amount of users in the event after bobby joined: 1
Address added onto the array: 1
user bobby calls (cancelParticipation):
bobby's assets before cancellation: 49250000000000000000
bobby's assets after cancellation: 0
THE BUGS:
The updated number of participations after user cancelled: (BUG) 1
Addresses exisitng in the array after user bobby cancelsParticipation: (BUG) 1
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.27ms (777.00µs CPU time)
Recommended Mitigation
function cancelParticipation() public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
uint256 refundAmount = stakedAsset[msg.sender];
+ uint256 shares = balanceOf(msg.sender);
stakedAsset[msg.sender] = 0;
+ delete userToCountry[msg.sender];
+
// Note: Removing from usersAddress array is gas-intensive
// Consider using a mapping to track cancelled users instead
+ numberOfParticipants--;
+ totalParticipantShares -= shares;
- uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
Alternative approach: Instead of removing from the array (expensive), add a mapping(address => bool) public cancelled and check it in _getWinnerShares() to skip cancelled users.
