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

Countries can be changed during and after event

neomartis

Countries can be changed during and after event

Description

  • Normal behavior: Once the event is started or finalized, game parameters (like the list of countries) should be immutable so that past selections and winner verification cannot be manipulated.

  • Issue: The owner can call setCountry at any time, even during the game or after the winner is set. The contract verifies withdrawals using string equality between userToCountry[msg.sender] and winner. By modifying country names post-finalization, the owner can create or influence string matches, enabling previously losing users to withdraw or otherwise causing confusion/inconsistency.

// @> no guard prevents changing countries after event start or after winner set
function setCountry(string[48] memory countries) public onlyOwner {
require(!_setWinner, "winner already set");
require(block.timestamp < eventStartDate, "event started");
...
}
​
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
// @> winner is set based on a string
winnerCountryId = countryIndex;
winner = teams[countryIndex];
...
}
​
​
function joinEvent(uint256 countryId) public {
...
// @> user’s picked country stored as string at join time
userToCountry[msg.sender] = teams[countryId];
...
}
​
function withdraw() external winnerSet {
...
if (
// @> withdraw verifies win by string equality rather than stable index
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
...
}

Risk

Likelihood: Low

  • Requires owner action post-finalization, but there is no on-chain restriction preventing it.

Impact: Medium

  • Allows out-of-sync states where users picked countries that cannot be selected as winners.

  • Allows post-finalization manipulation of outcome verification via mutable country names.

Proof of Concept

Description:

  • owner sets initial countries

  • user1 joins and picks country 7

  • owner changes the name of country 7

  • country 7 is picked as a winner

  • user1 cannot claim any rewards

function testSetCountryAfterWinnerSet() public {
// Set initial countries
vm.prank(owner);
briVault.setCountry(countries);
​
// User 1 deposits and joins
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user1);
briVault.joinEvent(7);
vm.stopPrank();
​
vm.startPrank(owner);
countries[7] = "New country 1";
briVault.setCountry(countries);
​
vm.warp(eventEndDate + 1);
​
// Set winner
vm.startPrank(owner);
briVault.setWinner(7);
vm.stopPrank();
​
vm.startPrank(user1);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
briVault.withdraw();
vm.stopPrank();
}

Recommended Mitigation

  • Disallow changing countries after event start or once a winner is set.

  • Prefer verifying by immutable indices rather than mutable strings (i.e., store and compare country IDs, not names) for withdrawals.

function setCountry(string[48] memory countries) public onlyOwner {
+ require(!_setWinner, "winner already set");
+ require(block.timestamp < eventStartDate, "event started");
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}

Index-based verification alternative (avoid string equality):

- if (keccak256(abi.encodePacked(userToCountry[msg.sender])) != keccak256(abi.encodePacked(winner))) {
- revert didNotWin();
- }
+ require(userSharesToCountry[msg.sender][winnerCountryId] != 0, "didNotWin");
Updates

Appeal created

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

setCountry() Can Be Called After Users Join

This is owner action.

Support

FAQs

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

Give us feedback!