BriVault

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

Centralized betting result decided by contract owner

Root + Impact

Description

  • The contract owner can set the winner of the tournament, he can bet on his decided winner

function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {}

Risk

Likelihood: Low

  • Contract owner bets on his favorite team, then at the end of the tournament, he set his choice to be the winner

Impact:

  • This is not fair for other players

  • Centralized risk: if the contract owner loses his private key, the attacker can decide his betting choice to be winner

Proof of Concept

  • PoC of owner betting and setting his choice to be the winner

  • Add this test case to briVault.t.sol

function testOwnerBetting() public {
vm.prank(owner);
briVault.setCountry(countries);
// other players bet
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
briVault.joinEvent(10);
vm.stopPrank();
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
briVault.joinEvent(20);
vm.stopPrank();
vm.startPrank(owner);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, owner);
briVault.joinEvent(2);
vm.warp(eventEndDate + 1);
string memory winner = briVault.setWinner(2);
console.log(winner);
string memory result = briVault.getWinner();
assertEq(result, "Mexico");
vm.stopPrank();
// owner withdraw his winner reward
assertEq(mockToken.balanceOf(owner), 15 ether);
vm.prank(owner);
briVault.withdraw();
assertEq(mockToken.balanceOf(owner), 29775000000000000000);
}

Recommended Mitigation

  • Use a decentralized oracle VRF like Chainlink VRF for winner decision

  • Use multisig wallet for contract owner to mitigate centralized risk

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!