Root + Impact
Description
## Root Cause
The `cancelParticipation()` function (lines 264-276) burns shares and refunds tokens but fails to clean critical state mappings:
```solidity
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);
}
```
After cancellation:
- `userSharesToCountry[user][countryId]` retains old share value
- `userToCountry[user]` still maps to the team
- User's address remains in `usersAddress[]`
- `numberOfParticipants` is not decremented
Risk
Likelihood:
Impact:
This creates "ghost participants" that corrupt the tournament state:
Scenario 1: Inflated winner calculations
Alice deposits 1000 tokens, joins country 5, then cancels (gets refund)
Her address stays in usersAddress[] with userSharesToCountry[alice][5] = 1000
When _getWinnerShares() runs, it iterates through usersAddress[] and adds her shares: totalWinnerShares += 1000
But Alice has 0 actual shares (they were burned)
This inflates totalWinnerShares, reducing payouts for legitimate winners
Scenario 2: Multiple participation
Bob deposits, joins country 3, cancels
Bob deposits again, joins country 3 again
Bob now has TWO entries in usersAddress[] both with potentially different share values
Combined with the multiple joinEvent() vulnerability, this amplifies the exploit
Scenario 3: Incorrect participant count
Proof of Concept
## Proof of Concept
```solidity
function testGhostParticipant() public {
vm.startPrank(alice);
token.approve(address(vault), 1000e18);
uint256 shares = vault.deposit(1000e18, alice);
vault.joinEvent(5);
vm.stopPrank();
assertEq(vault.balanceOf(alice), shares);
assertEq(vault.userSharesToCountry(alice, 5), shares);
assertEq(vault.numberOfParticipants(), 1);
vm.prank(alice);
vault.cancelParticipation();
assertEq(vault.balanceOf(alice), 0);
assertEq(vault.userSharesToCountry(alice, 5), shares);
assertEq(vault.numberOfParticipants(), 1);
assertEq(vault.userToCountry(alice), "Country5");
vm.warp(vault.eventEndDate() + 1);
vm.prank(owner);
vault.setWinner(5);
uint256 totalWinner = vault.totalWinnerShares();
assertEq(totalWinner, shares);
}
function testMultipleEntriesAfterCancel() public {
vm.startPrank(bob);
token.approve(address(vault), 2000e18);
vault.deposit(1000e18, bob);
vault.joinEvent(3);
vault.cancelParticipation();
vault.deposit(1000e18, bob);
vault.joinEvent(3);
vm.stopPrank();
assertEq(vault.numberOfParticipants(), 2);
}
```
**Expected vs Actual:**
- Expected: After cancel, all user state is cleaned
- Actual: Ghost entries remain, corrupting calculations
Recommended Mitigation
- remove this code
+ add this code
## Recommended Mitigation
Track participation explicitly and clean all state on cancellation:
```solidity
// Add new state variables
mapping(address => bool) public hasJoinedEvent;
mapping(address => uint256) public userCountryId;
// Modify joinEvent to track participation
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
require(!hasJoinedEvent[msg.sender], "Already joined");
hasJoinedEvent[msg.sender] = true;
userCountryId[msg.sender] = countryId;
// ... rest of existing code
}
// Fix cancelParticipation to clean state
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 participation state
if (hasJoinedEvent[msg.sender]) {
uint256 countryId = userCountryId[msg.sender];
// Clear mappings
delete userSharesToCountry[msg.sender][countryId];
delete userToCountry[msg.sender];
delete userCountryId[msg.sender];
hasJoinedEvent[msg.sender] = false;
// Decrement counter
numberOfParticipants--;
}
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
```
**Alternative approach - Use actual balance instead of stale state:**
```solidity
function _getWinnerShares() internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; i++) {
address user = usersAddress[i];
// Check if user actually has shares
uint256 userShares = balanceOf(user);
if (userShares > 0 &&
keccak256(abi.encodePacked(userToCountry[user])) ==
keccak256(abi.encodePacked(winner))) {
totalWinnerShares += userShares;
}
}
return totalWinnerShares;
}
```
This approach uses actual share balances instead of relying on potentially stale `userSharesToCountry` mapping.
