Root + Impact
Description
If the owner is the one who chooses the winning countryId, they should not be allowed to place a bet on an outcome that they can decide themselves.
In this case, there are no restrictions on the owner. Nothing prevents them from betting on a specific countryId and later selecting that same countryId as the winner, thereby guaranteeing their own victory.
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);
}
Risk
Likelihood:
Impact:
-
The owner can choose the countryId with the fewest shares, vote for it, and then declare that same countryId as the winner. In doing so, they not only harm the losing users by causing them to lose their funds, but also negatively impact the actual winners, as the rewards will be further diluted among a larger number of winning tokens.
-
The severity of this vulnerability is further increased because the owner has the ability to arbitrarily mint the token used as the underlying asset.
Proof of Concept
The setup is the same as the tests performed by the developer.
This test demonstrates how the owner is able to vote and then arbitrarily select their own countryId as the winner in order to claim the reward.
function testOwnerCanPartecipateAndSetHisTeamAsWinner() public {
uint256 depositAmount = 5 ether;
vm.startPrank(user1);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user1);
briVault.joinEvent(2);
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user2);
briVault.joinEvent(3);
vm.stopPrank();
vm.startPrank(owner);
uint256 ownerBalanceBeforeDeposit = mockToken.balanceOf(owner);
mockToken.approve(address(briVault), ownerBalanceBeforeDeposit);
briVault.deposit(ownerBalanceBeforeDeposit, owner);
briVault.joinEvent(1);
vm.warp(briVault.eventEndDate() +1);
briVault.setWinner(1);
vm.stopPrank();
uint256 totalVaultAsset = mockToken.balanceOf(address(briVault));
console.log("vault token balance: ", totalVaultAsset);
vm.prank(owner);
briVault.withdraw();
console.log("vault token balance after withdraw: ", mockToken.balanceOf(address(briVault)));
uint256 ownerFinalBalance = mockToken.balanceOf(owner);
console.log("owner final balance: ", ownerFinalBalance);
assertEq(ownerFinalBalance, totalVaultAsset);
}
Recommended Mitigation
To mitigate this issue, the owner should be prevented from betting when they have the ability to decide the winning countryId. The best approach is to use a modifier that prevents the owner from calling deposit() and joinEvent().
+ modifier ExceptOwner() {
+ if(msg.sender == owner){
+ revert ownerCanCallThisFunction();
+ }
+ _;
+ }
+ function deposit(uint256 assets, address receiver) public override returns (uint256) ExceptOwner {
....
}
+ function joinEvent(uint256 countryId) public ExceptOwner {
....
}