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

No checks in setCountry() for clearing/overwriting previous countries

s3mvl4d

Description

  • When the owner updates the list of countries for a new event/round, the contract should fully replace the previous set - clearing out any leftover entries so only the intended countries are selectable.

  • setCountry(string[48] memory countries) writes incoming entries into the fixed string[48] teams array using a simple for loop but does not zero out indices beyond the new list’s effective length. If a shorter (or partially empty) list is provided, old names remain at those higher indices and can still be selected later via joinEvent()/getCountry(), leading to unintended teams in the tournament.

function setCountry(string[48] memory countries) public onlyOwner {
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i]; // @> overwrites only provided indices
}
emit CountriesSet(countries);
}
// @> No explicit clearing of leftover entries in `teams`
// @> `getCountry(id)` only checks `bytes(teams[id]).length != 0`, so stale strings remain valid.

Risk

Likelihood: Low

  • Operations may pass a partially filled countries array (e.g., leaving trailing entries empty) or reuse the function with fewer active teams in a subsequent round, which leaves previous non‑empty names intact at higher indices.

Impact: Low

  • Unintended team selection: Users can call joinEvent() with an index where stale team names persist, participating in a team that should not exist for the current event.

  • Accounting/payout inconsistencies: Winner selection (setWinner(countryIndex)) could target a stale team or misalign with front‑end assumptions, corrupting winnerCountryId and downstream share sums.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test} 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";
​
contract SetCountryLeftoversTest is Test {
BriVault vault;
MockERC20 token;
address owner = makeAddr("owner");
uint256 start; uint256 end;
​
function setUp() public {
start = block.timestamp + 2 days;
end = start + 30 days;
token = new MockERC20("Mock", "M");
vault = new BriVault(IERC20(address(token)), 150, start, makeAddr("fee"), 0.0002 ether, end);
}
​
function test_LeftoverTeamsRemain() public {
// Round 1: set many countries (index 0..10 filled)
string[48] memory c1;
c1[0] = "Japan"; c1[1] = "Spain"; c1[10] = "Mexico";
vm.prank(owner); vault.setCountry(c1);
assertEq(vault.getCountry(10), "Mexico");
​
// Round 2: set a shorter/partial list (only index 0..1 filled, index 10 left empty in input)
string[48] memory c2;
c2[0] = "France"; c2[1] = "Germany";
vm.prank(owner); vault.setCountry(c2);
​
// Because setCountry doesn’t clear leftovers, index 10 still returns "Mexico"
assertEq(vault.getCountry(10), "Mexico"); // ← stale entry remains selectable
}
}

Recommended Mitigation

  • Clear all previous entries before writing the new list—or, track an explicit active length and enforce it in selection.

function setCountry(string[48] memory countries) public onlyOwner {
- for (uint256 i = 0; i < countries.length; ++i) {
- teams[i] = countries[i];
- }
+ // Clear all previous entries to avoid stale names
+ for (uint256 i = 0; i < teams.length; ++i) {
+ teams[i] = "";
+ }
+ // Set new countries
+ for (uint256 i = 0; i < countries.length; ++i) {
+ 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!