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

`setCountry` can be called any time during the tournament, excluding already joined participants of undesired countries but still holding their deposits

valya

Root + Impact

Description

  • Participants expect to have a chance to win a prize once they deposited and joined an event using some countryId.

  • Participants who joined an event using a countryId that an owner does not like may be silently excluded from an event by the owner, but their deposits will still be in the protocol, and once an event started they will not be able to cancel participation even if they knew about this possibility earlier.

@>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: HIGH

  • An owner can change the set of countries at any time during the tournament.

Impact: HIGH

  • Depositors are silently excluded from an event if their country of choice is not in the set anymore.

  • The funds of excluded participants are still in the protocol, and depositors lose them permanently.

Proof of Concept

  1. Set countries before the event start.

  2. Set other desired countries after eventStartDate but before eventEndDate.

  3. Set other desired countries after eventEndDate.

Add the following test to briVault.t.sol:

function test_setCountry_multipleTimes_anyTime() public {
vm.startPrank(owner);
briVault.setCountry(countries);
assertEq(briVault.teams(2), "Mexico");
​
// set other desired countries after start of event but before end of event
countries[0] = "";
countries[47] = "";
vm.warp(block.timestamp + eventStartDate + 1);
vm.roll(block.number + 1);
​
briVault.setCountry(countries);
assertEq(briVault.teams(0), "");
assertEq(briVault.teams(47), "");
​
// set other desired countries after enf of event
countries[1] = "";
vm.warp(block.timestamp + eventEndDate + 1);
vm.roll(block.number + 1);
​
briVault.setCountry(countries);
assertEq(briVault.teams(1), "");
}

Recommended Mitigation

  1. Set teams once in a constructor only, and remove the public function setCountry:

constructor (IERC20 _asset, uint256 _participationFeeBsp, uint256 _eventStartDate, address _participationFeeAddress, uint256 _minimumAmount, uint256 _eventEndDate, string[48] memory _countries) ERC4626 (_asset) ERC20("BriTechLabs", "BTT") Ownable(msg.sender) {
if (_participationFeeBsp > PARTICIPATIONFEEBSPMAX){
revert limiteExceede();
}
+ teams = _countries;
participationFeeBsp = _participationFeeBsp;
eventStartDate = _eventStartDate;
eventEndDate = _eventEndDate;
participationFeeAddress = _participationFeeAddress;
minimumAmount = _minimumAmount;
_setWinner = false;
}
​
​
-function setCountry(string[48] memory countries) public onlyOwner {
- for (uint256 i = 0; i < countries.length; ++i) {
- teams[i] = countries[i];
- }
- emit CountriesSet(countries);
-}

  1. Alternatively, add a check on teams.length when setCountry is called and revert once teams are already set:

+ error TeamsAlreadySet();
​
function setCountry(string[48] memory countries) public onlyOwner {
+ if (teams.length > 0) {
+ revert TeamsAlreadySet();
+ }
for (uint256 i = 0; i < countries.length; ++i) {
teams[i] = countries[i];
}
emit CountriesSet(countries);
}
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!