Submission Details
Severity:
CancelParticipation doesn't really withdraw user from the tournament
Root + Impact
Description
-
BriVault::cancelParticipation()function doesn't really withdraw the user from the tournament. It only removes the user share into 0 -
If the user country is win, the other winner share will be reduce by user previous share
// Root cause in the BriVault.sol
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:
This will always happen.
Impact:
If the user bet on the country winner, the other user who bet on the same country will have reduce rewards
Proof of Concept
To see such scenario happen, we can put this test case function on BriVault.t.sol
function testCancelParticipation() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
uint256 user1Shares = briVault.deposit(5 ether, user1);
briVault.joinEvent(9);
uint256 balanceBeforuser1 = mockToken.balanceOf(user1);
vm.stopPrank();
// user2 betting on Chile but cancel participation & get refunded
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
uint256 user2Shares = briVault.deposit(5 ether, user2);
briVault.joinEvent(9);
uint256 balanceBeforuser2 = mockToken.balanceOf(user2);
briVault.cancelParticipation();
uint256 balanceAfteruser2 = mockToken.balanceOf(user2);
console.log("user2 get refund: ", balanceAfteruser2 - balanceBeforuser2);
vm.stopPrank();
vm.startPrank(user3);
mockToken.approve(address(briVault), 5 ether);
uint256 user3Shares = briVault.deposit(5 ether, user3);
briVault.joinEvent(30);
vm.stopPrank();
vm.startPrank(user4);
mockToken.approve(address(briVault), 5 ether);
uint256 user4Shares = briVault.deposit(5 ether, user4);
briVault.joinEvent(10);
uint256 balanceBeforuser4 = mockToken.balanceOf(user4);
vm.stopPrank();
console.log("user1 share: ", user1Shares);
console.log("user1 country ", briVault.userToCountry(user1));
console.log("user2 share: ", user2Shares);
console.log("user2 country ", briVault.userToCountry(user2));
console.log("user3 share: ", user3Shares);
console.log("user3 country ", briVault.userToCountry(user3));
console.log("user4 share: ", user4Shares);
console.log("user4 country ", briVault.userToCountry(user4));
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(9);
console.log("total price is: ", briVault.finalizedVaultAsset());
console.log("winner is: ", briVault.getWinner());
console.log("total winner share: ", briVault.totalWinnerShares());
vm.stopPrank();
vm.startPrank(user1);
briVault.withdraw();
uint256 balanceAfterUser1 = mockToken.balanceOf(user1);
console.log("withdraw amount for user1: ", balanceAfterUser1 - balanceBeforuser1);
vm.stopPrank();
vm.startPrank(user2);
uint256 balanceBefore2 = mockToken.balanceOf(user2);
briVault.withdraw();
uint256 balanceAfter2 = mockToken.balanceOf(user2);
console.log("withdraw amount: ", balanceAfter2 - balanceBefore2);
vm.stopPrank();
vm.startPrank(user3);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
briVault.withdraw();
vm.stopPrank();
vm.startPrank(user4);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
briVault.withdraw();
vm.stopPrank();
}
if we run 'forge test --match-test testCancelParticipation -vvvv' from the terminal we can see this log output
Logs:
user2 get refund: 4925000000000000000
user1 share: 4925000000000000000
user1 country Chile
user2 share: 4925000000000000000
user2 country Chile
user3 share: 4925000000000000000
user3 country Serbia
user4 share: 4925000000000000000
user4 country Japan
total price is: 14775000000000000000
winner is: Chile
total winner share: 9850000000000000000
withdraw amount for user1: 7387500000000000000
withdraw amount: 0 // this is withdraw for user2
From log we can see that user2 already get refunded 4.925 x 1e18.
But when the winner set for Chile, user2 still in position, still have shares and eligible as winner.
Total winner shares showing there are 2 winner : 9.85 = 4.925 x 2.
User1 only get half of the total price : 7.3877 = 14.775 / 2
Recommended Mitigation
Add helper function BriVault::removeUser() and called it from BriVault::cancelParticipation() before safeTransfer
+ mapping (address => uint256) public userToCountryid;
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
// Ensure countryId is a valid index in the `teams` array
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
userToCountry[msg.sender] = teams[countryId];
+ userToCountryid[msg.sender] = countryId;
......
}
+function removeUser(
+ address user, uint256 shares
+ ) internal {
+ uint256 userCountryId = userToCountryid[msg.sender];
+ delete userSharesToCountry[msg.sender][userCountryId];
+ delete userToCountry[msg.sender];
+ numberOfParticipants--;
+ totalParticipantShares -= shares;
+ // @usersAddress variable suggest to be changed into mapping to get more gas efficient,
+ // when add & removing
+ for (uint i = 0; i < usersAddress.length - 1; i++) {
+ if (usersAddress[i] == user){
+ delete usersAddress[i];
+ }
+ }
+ }
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);
+ removeUser(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, refundAmount);
}
Updates
Appeal created
bube 19 days ago
Submission Judgement Published Assigned finding tags:
`cancelParticipation` Leaves Stale Winner Data
CancelParticipation burns shares but leaves the address inside usersAddress and keeps userSharesToCountry populated.
Rewards breakdown
nSLOC
193This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.