Root + Impact
Description
-
When a user participates in an event and then, calls the BriVault::cancelParticipation function, this function fails to make proper changes to the state variables to reflect removal of the user from the event.
-
Therefore, it causes miscalculations which affect the winners' share of the vault.
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
@> userSharesToCountry[msg.sender][countryId] = participantShares;
@> usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}
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);
}
function _getWinnerShares () internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){
@> address user = usersAddress[i];
@> totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}
Risk
Likelihood: High
-
This will occur when a user depoists some fund, joins an event, and then calls the BriVault::cancelParticipation function.
-
since these are common functionalities of the protocol, it is quite likely to happen.
Impact:
-
It can break the functionality of the protocol by miscalculating the winners' prizes.
-
It also leads to remaining of some funds in the vault when all winners have withdrawn their wrongly calculated prize shares.
Proof of Concept
Please copy/paste the following function into the test file and run it.
function test_cancelParticipationCanCausePrizeMiscalculations() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(10);
briVault.cancelParticipation();
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
briVault.joinEvent(10);
vm.stopPrank();
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
vm.stopPrank();
uint256 user2BalanceBefore = IERC20(address(mockToken)).balanceOf(
user2
);
uint256 vaultBalanceBefore = IERC20(address(mockToken)).balanceOf(
address(briVault)
);
vm.startPrank(user2);
briVault.withdraw();
vm.stopPrank();
uint256 user2BalanceAfter = IERC20(address(mockToken)).balanceOf(user2);
uint256 vaultBalanceAfter = IERC20(address(mockToken)).balanceOf(
address(briVault)
);
assert(user2BalanceAfter - user2BalanceBefore < 3 ether);
assert(vaultBalanceBefore - vaultBalanceAfter > 0);
}
Recommended Mitigation
To solve the isse please make the following changes.
function cancelParticipation () public {
if (block.timestamp >= eventStartDate){
revert eventStarted();
}
+ address user;
+ for (uint256 i = 0; i < usersAddress.length; ++i) {
+ user = usersAddress[i];
+ if (user == msg.sender) {
+ usersAddress[i] = usersAddress[usersAddress.length - 1];
+ usersAddress.pop();
+ userSharesToCountry[user][
+ getCountryId(userToCountry[user])
+ ] = 0;
+ break;
+ }
+ }
uint256 refundAmount = stakedAsset[msg.sender];
stakedAsset[msg.sender] = 0;
uint256 shares = balanceOf(msg.sender);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
+ function getCountryId(string memory country) public view returns (uint256) {
+ for (uint256 i = 0; i < teams.length; i++) {
+ if (
+ keccak256(abi.encodePacked(teams[i])) ==
+ keccak256(abi.encodePacked(country))
+ ) {
+ return i;
+ }
+ }
+ revert invalidCountry();
+ }
