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

Centralization risk in `setWinner::BriVault` function

elo_anxiety

Root + Impact : setWinner::BriVault is onlyOwner function, dishonest owner can set wrong country winner hence distroying the whole protocol.

Description

  • setWinner function is onlyOwner ,hence owner can set a wrong winner in order to gain incentive.

@> function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
​
require(countryIndex < teams.length, "Invalid country index");
​
if (_setWinner) {
revert WinnerAlreadySet();
}
​
winnerCountryId = countryIndex;
winner = teams[countryIndex];
​
_setWinner = true;
​
_getWinnerShares();
​
_setFinallizedVaultBalance();
​
emit WinnerSet(winner);
​
return winner;
}

Risk

Likelihood: HIGH

  • It is highly possible that some dishonest admin can manipulate the result of the event.

Impact:

  • Loss of funds for users who have bet on the actual winner country .

Proof of Concept

  • All the users have joined the event and the event started.

  • owner has also joined the event .

  • Suppose there are 3 users for example with the countries they have bet on-> 1. owner(United States) 2. alice(Canada) 3. bob(Mexico).

  • All three have deposited 5e18 .

  • In actual, Canada won the event and the event is ended.

  • But the owner bet on United States ,so admin will call the function setWinner but with countryIndex as 0 .

function testWinnerCanBeChangedByAdmin() public {
//owner Balance Initially
uint256 ownerAssetBalanceBefore = IERC20(briVault.asset()).balanceOf(owner);
uint256 fee = (5e18 * 150) / 10000;
​
//owner set the country list
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
//owner deposited and joined betting for US
vm.startPrank(owner);
mockToken.approve(address(briVault), 5 ether);
uint256 ownerShares = briVault.deposit(5 ether, owner);
briVault.joinEvent(0); //US
console.log("owners's shares", ownerShares);
vm.stopPrank();
​
//alice deposited and joined betting for Canada
vm.startPrank(alice);
mockToken.approve(address(briVault), 5 ether);
uint256 aliceShares = briVault.deposit(5 ether, alice);
briVault.joinEvent(1); //CANADA
console.log("alice shares", aliceShares);
vm.stopPrank();
​
//bob deposited and joined betting for Mexico
vm.startPrank(bob);
mockToken.approve(address(briVault), 5 ether);
uint256 bobShares = briVault.deposit(5 ether, bob);
vm.warp(eventStartDate); // last user to enter
briVault.joinEvent(2); //MEXICO
console.log("bob's shares", bobShares);
vm.stopPrank();
//now no one can join the event.
​
//Event ended and CANADA is the winner, hence alice should get the winning funds back.
vm.warp(eventEndDate + 1);
//owner changed the winner
vm.startPrank(owner);
briVault.setWinner(0); //US IS THE WINNER NOW.Hence alice won't be able to withdraw and will lose his assets, while owner rug pulled the users.
string memory winnerCountry = briVault.getWinner();
console.log("winner country:", winnerCountry); //US
vm.stopPrank();
​
//alice can't withdraw
vm.expectRevert(BriVault.didNotWin.selector);
vm.prank(alice);
briVault.withdraw();
​
vm.prank(owner);
briVault.withdraw();
uint256 ownerAssetBalanceAfter = IERC20(briVault.asset()).balanceOf(owner);
console.log("OWNER Balance After winning:", ownerAssetBalanceAfter);
assertEq(ownerAssetBalanceBefore - 5e18 + (5e18 - fee) * 3, ownerAssetBalanceAfter); //calculation and comparision on gains by owner
}

Recommended Mitigation

  • The design of the contract makes it difficult to avoid the Centralization Issue.

  • Change the design of the contract and make it a Governance Protocol, where a set of event officials vote for the correct result .

  • If making a governance protocol, should set the quorum to 100%.

Updates

Appeal created

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

The winner is set by the owner

This is owner action and the owner is assumed to be trusted and to provide correct input arguments.

Support

FAQs

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

Give us feedback!