BriVault

First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Impact: high
Likelihood: high
Invalid

Owner can both partecipate and choose the winner

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 {
@> //no restriction to avoid owner bet
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];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}

Risk

Likelihood:

  • The vulnerability can be exploited directly by the contract owner without requiring any special conditions.

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 {
//owner can both partecipate and choose the winner
uint256 depositAmount = 5 ether;
vm.startPrank(user1);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user1);
briVault.joinEvent(2); // joining with countryId 2
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), depositAmount);
briVault.deposit(depositAmount, user2);
briVault.joinEvent(3); // joining with countryId 3
vm.stopPrank();
vm.startPrank(owner);
uint256 ownerBalanceBeforeDeposit = mockToken.balanceOf(owner);
mockToken.approve(address(briVault), ownerBalanceBeforeDeposit);
briVault.deposit(ownerBalanceBeforeDeposit, owner);
briVault.joinEvent(1); // joining with countryId 1
vm.warp(briVault.eventEndDate() +1);
briVault.setWinner(1); // setting winner to countryId 1
vm.stopPrank();
// total vault assets
uint256 totalVaultAsset = mockToken.balanceOf(address(briVault));
console.log("vault token balance: ", totalVaultAsset);
//owner withdraw
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);
// owner should receive all vault assets
// owner deposited full balance so initial balance is 0
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 {
....
}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

The owner can be participant

The owner is trusted.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!