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

The Centralized Owner Can Steal Participant Funds by Designating Themselves as the Winner.

kadejo

The Centralized Owner Can Steal Participant Funds by Designating Themselves as the Winner.

Description

  • The protocol contract designates the owner as the sole authority to set the outcome of an event via the briVault::setWinner() function.

  • However, there is no restriction preventing the owner (or an address controlled by them) from participating in the same event using briVault::joinEvent().

@> function joinEvent(uint256 countryId) public {

Risk

Likelihood:

  • Reason 1 Whenever the user owner wishes, they have the authority to assign the winner and participate in the event.

Impact:

  • Impact 1 This design flaw breaks the protocol's trust model and allows the owner to guarantee their victory 100% of the time.

  • Impact 2 A malicious owner can wait for users to deposit funds into the event, participate themselves, and then simply call briVault::setWinner() with the team they chose, thereby draining all the funds staked by other participants into their own account.


Proof of Concept

The following proof of concept simulates a real-world exploitation scenario:

  1. Victim (userA) deposits funds and bets on team 1.

  2. Attacker (owner) deposits a smaller amount of funds and bets on team 9.

  3. The Attacker calls setWinner(9), declaring the team they bet on as the winner.

  4. The Attacker withdraws their winnings, which include the victim's funds.

  5. The final asserts confirm that the owner's final balance is greater than their initial deposit (proving fraudulent profit) and that the victim's balance is zero (proving the loss).

function test_OwnerCanRugParticipantsByWinning() public {
// 1. Set up a victim participant (userA)
address userA = address(0x1);
vm.startPrank(userA);
mockToken.mint(userA, 10 ether);
mockToken.approve(address(briVault), 10 ether);
briVault.deposit(10 ether, userA);
briVault.joinEvent(1); // userA bets on team 1
vm.stopPrank();
​
// 2. The owner joins and bets on a different team
vm.startPrank(owner);
briVault.setCountry(countries);
mockToken.approve(address(briVault), 5 ether);
uint256 ownerShares = briVault.deposit(5 ether, owner);
briVault.joinEvent(9); // owner bets on team 9
​
// 3. The owner finalizes the event and declares themselves the winner
vm.warp(eventEndDate + 1);
briVault.setWinner(9);
vm.stopPrank();
​
// 4. Verify the impact (Owner claims winnings)
vm.startPrank(owner);
briVault.withdraw();
vm.stopPrank();
​
​
uint256 ownerFinalBalance = mockToken.balanceOf(owner);
uint256 userAFinalBalance = mockToken.balanceOf(userA);
​
// Assert that the owner won more than they put in (5 ether)
assertTrue(ownerFinalBalance > 5 ether);
​
// Assert that userA lost everything (or did not gain anything)
assertEq(userAFinalBalance, 0);
​
// Verify that the 'winner' is the owner's choice
string memory actualWinner = briVault.getWinner();
assertEq(actualWinner, countries[9], unicode"Chile");
}

Recommended Mitigation

To prevent the exact owner address from participating, a check should be added to the briVault::joinEvent() function:

function joinEvent(uint256 countryId) public {
+ if(msg.sender == owner) {
+ revert OwnerCannotParticipate();
+ }
...
}

Alternative Recommendation:

The root problem is the centralization of the briVault::setWinner() function. The protocol will never be truly decentralized or trustless as long as a single entity decides the results.

The recommended solution is to completely remove the briVault::setWinner() function and replace it with a decentralized mechanism, such as an oracle. For example, using a service like Chainlink Data Feeds would allow the contract to fetch event results from reliable on-chain sources.

Updates

Appeal created

bube Lead Judge 21 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.

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!