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

Owner Can Change Teams After Users Join Event

0xsnoweth

Root + Impact

Description

  • Normal Behavior: ThesetCountry() function should only be callable during the setup phase before any users have deposited and joined the event. Once users have selected teams via joinEvent(), the team mappings should be immutable to ensure fairness.


  • Vulnerability: The setCountry() function has no access control preventing the owner from calling it after users have already joined the event. When the owner changes the teams array after users have bet, it creates a mismatch between the team names stored in userToCountry[user] and the actual team names in the teams array at those indices.

function setCountry(string[48] memory countries) public onlyOwner {
// @> No check preventing this from being called after users have joined
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}

Risk

Likelihood: HIGH

  • Reason 1: Any time a malicious or compromised owner wants to manipulate the outcome after seeing which teams users have bet on, they can call setCountry() to change team mappings.


  • Reason 2: Any time users have joined the event and the owner calls setCountry() for any reason (even accidentally), all existing user bets become invalid.

Impact: HIGH

  • Impact 1: Users who bet on the "winning" index cannot withdraw their winnings because their stored team name in userToCountry[user] no longer matches the winner string after the owner changes the teams array.


  • Impact 2: The owner can effectively steal all user funds by changing team mappings after users join, ensuring no user's stored team matches the declared winner, making all deposits unrecoverable.


  • Impact 3: Complete loss of trust in the protocol - users have no guarantee their bets will remain valid after joining.

Proof of Concept

You may copy and paste the below POC on the existing test suite:

function test_OwnerCanChangeTeamsAfterUsersJoin() public {
// Owner sets countries (index 10 = "Japan")
vm.startPrank(owner);
briVault.setCountry(countries);
string memory result = briVault.getCountry(10);
console.log("Country the owner has set for index 10 initially: ", result);
console.log();
console.log("*user1 calls joinEvent function with index 10*");
console.log();
vm.stopPrank();
​
address user1 = makeAddr("user1");
mockToken.mint((user1), 50 ether);
// User deposits and joins event with countryId 10
vm.startPrank(user1);
mockToken.approve(address(briVault), type(uint256).max);
​
briVault.deposit(50 ether, user1);
​
briVault.joinEvent(10);
string memory user1Team = briVault.userToCountry(user1);
console.log("Team user1 supports after joining event is 10th index: ", user1Team);
vm.stopPrank();
​
// Owner calls setCountry() again, changing index 10 to "Argentina"
vm.startPrank(owner);
​
// we are making a new modified 48 countries array where index 10 is now modified
string[48] memory modifiedCountries = countries;
modifiedCountries[10] = "Argentina";
​
//now we set the new modified countries.
briVault.setCountry(modifiedCountries);
string memory newResult = briVault.getCountry(10);
console.log();
console.log("The newly modified 10th index country is: ", newResult);
vm.stopPrank();
​
// Owner sets winner to index 10 ("Argentina") after event finished
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
string memory teamWinners = briVault.getWinner();
console.log();
console.log("The country team that won after event ended: ", teamWinners);
vm.stopPrank();
​
// User's userToCountry[user] = "Japan" doesn't match winner
string memory usersToCountry = briVault.userToCountry(user1);
console.log("The country user1's 10th index that was suppose to win: (BUG)", usersToCountry);
assertNotEq(usersToCountry, teamWinners);
​
// User can't withdraw despite betting on the "winning" index
vm.startPrank(user1);
vm.expectRevert("didNotWin()");
briVault.withdraw();
}
​

Output Confirmation:

[PASS] test_OwnerCanChangeTeamsAfterUsersJoin() (gas: 2024222)
Logs:
Country the owner has set for index 10 initially: Japan
*user1 calls joinEvent function with index 10*
Team user1 supports after joining event is 10th index: Japan
The newly modified 10th index country is: Argentina
The country team that won after event ended: Argentina
The country user1's 10th index that was suppose to win: (BUG) Japan
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.41ms (1.90ms CPU time)

Recommended Mitigation

Add a check in setCountry() to prevent it from being called after users have joined:

function setCountry(string[48] memory countries) public onlyOwner {
+ if (numberOfParticipants > 0 || block.timestamp >= eventStartDate) {
+ revert("Cannot change teams after users have joined or event started");
+ }
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}

This ensures team mappings are immutable once any user has joined the event, preserving the integrity of all user bets.

Updates

Appeal created

bube Lead Judge 20 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!