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

setCountry() Can Be Called After Users Join, Permanently Locking Funds

calvinkimani

Description

The missing timing restriction in BriVault.sol::setCountry() will cause permanent loss of funds for users as the owner will change the teams array after users have joined, creating a mismatch between name-based and index-based bet tracking that prevents withdrawals.

In BriVault.sol:106-111, the setCountry() function has no timing restrictions or checks for existing participants:

function setCountry(string[48] memory countries) public onlyOwner {
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}

Risk

Likelihood:

  1. Owner needs to call setCountry() to initialize teams array

  2. At least one user needs to call deposit() and joinEvent(countryId) to bet on a team

  3. Owner needs to call setCountry() again with modified teams array

  4. Event must end and owner must call setWinner() with an index whose team name changed

Impact

  • The affected user suffers a 100% loss of their deposited funds, which become permanently locked in the contract.

Additionally, if other users legitimately bet on the winning team:

  • Their payouts are reduced because totalWinnerShares includes the locked user's shares

  • Payout formula: assetToWithdraw = (userShares * vaultBalance) / totalWinnerShares

  • The difference remains permanently locked in the contract

Proof of Concept

Owner changes teams array after user joins, creating mismatch between name-based (userToCountry) and index-based (userSharesToCountry) tracking.
User's shares are counted in totalWinnerShares but they cannot withdraw.

function test_LockedFunds_SetCountryAfterJoin() public {
// Victim deposits and joins betting on Brazil at index 0
vm.startPrank(victim);
token.approve(address(vault), 10 ether);
vault.deposit(10 ether, victim);
vault.joinEvent(0); // Bet on Brazil (index 0)
vm.stopPrank();
​
uint256 victimShares = vault.balanceOf(victim);
string memory originalTeam = vault.userToCountry(victim);
​
// Verify victim bet on Brazil at index 0
assertEq(vault.teams(0), "Brazil", "Index 0 should be Brazil");
assertEq(originalTeam, "Brazil", "Victim should have bet on Brazil");
​
// Owner maliciously swaps teams in the array
string[48] memory newCountries;
newCountries[0] = "Argentina"; // SWAP: Argentina now at index 0
newCountries[1] = "Brazil"; // SWAP: Brazil now at index 1
newCountries[2] = "France";
newCountries[3] = "Germany";
for (uint256 i = 4; i < 48; i++) {
newCountries[i] = countries[i];
}
​
vm.prank(owner);
vault.setCountry(newCountries);
​
// Verify teams were swapped
assertEq(vault.teams(0), "Argentina", "Index 0 should now be Argentina");
assertEq(vault.teams(1), "Brazil", "Index 1 should now be Brazil");
​
// Fast forward to after event
vm.warp(eventEndDate + 1);
​
// Owner sets winner to index 0 (which is now Argentina)
vm.prank(owner);
vault.setWinner(0);
​
uint256 totalWinnerShares = vault.totalWinnerShares();
​
// Victim's shares were counted in totalWinnerShares (index-based)
assertEq(totalWinnerShares, victimShares, "Victim shares should be counted in totalWinnerShares");
​
// But victim cannot withdraw due to name mismatch (name-based check)
vm.prank(victim);
vm.expectRevert(BriVault.didNotWin.selector);
vault.withdraw();
​
// Verify the mismatch
string memory winner = vault.getWinner();
string memory victimTeam = vault.userToCountry(victim);
assertEq(winner, "Argentina", "Winner should be Argentina");
assertEq(victimTeam, "Brazil", "Victim's userToCountry should still be Brazil");
assertTrue(
keccak256(abi.encodePacked(winner)) != keccak256(abi.encodePacked(victimTeam)),
"Mismatch prevents withdrawal"
);
}

Recommended Mitigation

Add a timing restriction to setCountry() to prevent modifications after users have joined:

function setCountry(string[48] memory countries) public onlyOwner {
+ // Prevent changing teams after users have joined
+ if (numberOfParticipants > 0) {
+ revert CannotChangeTeamsAfterJoins();
+ }
+
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}

Alternatively, store the complete team configuration when users join to prevent inconsistency:

struct Bet {
string teamName;
uint256 teamIndex;
uint256 shares;
}
mapping(address => Bet) public userBets;
Updates

Lead Judging Commences

bryanconquer Auditor
25 days ago

Appeal created

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