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

M01. Global Winner Flag Locks All Country Results (Single-Winner Logic Flaw)

accessdenied

Root + Impact

Description

  • Normally, each country in the tournament is expected to have an independent outcome, as users can join by selecting a specific countryId in joinEvent(). The vault should ideally support multiple country results or isolated events.

  • However, the contract defines a single _setWinner boolean and a single winnerCountryId, which apply globally. Once the first winner is set, all subsequent attempts to finalize a different country revert, permanently locking the system to a single winner.

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:

  • The issue occurs at the end of every tournament when the owner attempts to set winners for more than one country.

  • Any additional call to setWinner() after the first successful one will revert, blocking finalization for other teams.

Impact:

  • Only one country can ever be marked as the winner across all participants.

  • Users who joined other countries become permanently unable to withdraw, resulting in locked funds and an inconsistent state.

Proof of Concept

function testMultipleCountryWinners() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.warp(eventEndDate + 1);
​
// First call succeeds
string memory firstWinner = briVault.setWinner(2);
assertEq(firstWinner, "Mexico");
​
// Second call fails because _setWinner is already true
vm.expectRevert(abi.encodeWithSignature("WinnerAlreadySet()"));
briVault.setWinner(5); // Attempt to set "Brazil" as another winner
vm.stopPrank();
}
​

Explanation:

  • The first call to setWinner(2) successfully sets “Mexico” as the global winner.

  • When attempting to call setWinner(5), the function reverts due to the _setWinner boolean already being true.

  • This demonstrates that all other countries are locked out, and only participants of the first winning country can withdraw their stakes.

Recommended Mitigation

To allow independent winner tracking per country, _setWinner and winner variables should be mapped by countryId instead of being global.

- bool public _setWinner;
- uint256 public winnerCountryId;
- string public winner;
+ mapping(uint256 => bool) public countryWinnerSet;
+ mapping(uint256 => string) public countryWinners;
​

Then update the logic in setWinner():

- if (_setWinner) {
- revert WinnerAlreadySet();
- }
- winnerCountryId = countryIndex;
- winner = teams[countryIndex];
- _setWinner = true;
+ if (countryWinnerSet[countryIndex]) {
+ revert WinnerAlreadySet();
+ }
+ countryWinners[countryIndex] = teams[countryIndex];
+ countryWinnerSet[countryIndex] = true;
​
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!