Description
The `cancelParticipation()` function allows users to cancel their participation and receive a refund of their staked assets. The function correctly clears the `stakedAsset` mapping and burns the user's shares. However, it does not clean up the data structures used by `joinEvent()`, specifically the `usersAddress` array and the `userSharesToCountry` mapping.
When users cancel their participation, they remain in the `usersAddress` array and their `userSharesToCountry` entries are not cleared. When the winner is determined, `_getWinnerShares()` iterates over all entries in `usersAddress` and counts shares for each user who bet on the winning country. Since cancelled users are still in the array, their shares (which no longer exist) are counted in `totalWinnerShares`, diluting the prize pool for winners.
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**:
* Users can cancel their participation before the event starts
* Cancelled users remain in the `usersAddress` array
* Winner calculation iterates over all entries including cancelled users
**Impact**:
* Prize pool dilution for legitimate winners
* Cancelled users' non-existent shares are counted in `totalWinnerShares`
* Legitimate winners receive less than their fair share
* Incorrect accounting of winner shares
Proof of Concept
In the following PoC test, user2 receives less than they should for winning due to user1's cancellation not having cleaned up the usersAddress array and the userSharesToCountry mapping.
function testCancelParticipationDoesntCleanup() public {
vm.prank(owner);
briVault.setCountry(countries);
vm.startPrank(user1);
mockToken.approve(address(briVault), 10 ether);
uint256 depositAmount = 5 ether;
uint256 shares = briVault.deposit(depositAmount, user1);
briVault.joinEvent(10);
briVault.cancelParticipation();
assertEq(briVault.balanceOf(user1), 0, "Shares should be burned");
assertEq(briVault.stakedAsset(user1), 0, "Staked asset should be cleared");
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 10 ether);
uint256 user2Shares = briVault.deposit(5 ether, user2);
briVault.joinEvent(10);
vm.stopPrank();
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
uint256 totalWinnerShares = briVault.totalWinnerShares();
console2.log("Total winner shares (includes cancelled user):", totalWinnerShares);
uint256 expectedWinnerShares = user2Shares;
uint256 actualWinnerShares = totalWinnerShares;
console2.log("Expected winner shares (only user2):", expectedWinnerShares);
console2.log("Actual winner shares (includes cancelled user1):", actualWinnerShares);
assertEq(actualWinnerShares, shares + user2Shares);
assertNotEq(actualWinnerShares, expectedWinnerShares);
vm.stopPrank();
uint256 balanceBefore = mockToken.balanceOf(user2);
vm.prank(user2);
briVault.withdraw();
uint256 balanceAfter = mockToken.balanceOf(user2);
uint256 received = balanceAfter - balanceBefore;
uint256 vaultAssets = briVault.finalizedVaultAsset();
uint256 expectedAmount = (user2Shares * vaultAssets) / expectedWinnerShares;
console2.log("User2 received:", received);
console2.log("User2 expected should have received:", expectedAmount);
assertLt(received, expectedAmount);
}
Recommended Mitigation
Clean up when `cancelParticipation()` is called.
+ mapping(address => uint256) public userIndexInArray; // Track user's index in usersAddress
function joinEvent(uint256 countryId) public {
// ... existing code ...
usersAddress.push(msg.sender);
+ userIndexInArray[msg.sender] = usersAddress.length - 1;
// ... rest of function ...
}
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);
+ // Clean up joinEvent data
+ uint256 userIndex = userIndexInArray[msg.sender];
+ if (userIndex < usersAddress.length) {
+ // Move last element to user's position and remove last
+ address lastUser = usersAddress[usersAddress.length - 1];
+ usersAddress[userIndex] = lastUser;
+ userIndexInArray[lastUser] = userIndex;
+ usersAddress.pop();
+ }
+
+ // Clear mappings
+ delete userSharesToCountry[msg.sender];
+ delete userToCountry[msg.sender];
+ delete userIndexInArray[msg.sender];
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
