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

[L-1] The `briVault::setWinner` does not follow the Checks, Effects and Interactions (CEI) pattern

0xliltee

[L-1] The briVault::setWinner does not follow the Checks, Effects and Interactions (CEI) pattern

Description

The `briVault::setWinner` violates the checks effects and interactions pattern by updating the winner be for setting the winner to `true`
​
```solidity
​
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
require(countryIndex < teams.length, "Invalid country index");
​
if (_setWinner) {
revert WinnerAlreadySet();
}
​
@> winnerCountryId = countryIndex;
@> winner = teams[countryIndex];
​
@> _setWinner = true;
​
_getWinnerShares();
​
_setFinallizedVaultBalance();
​
emit WinnerSet (winner);
return winner;
​
}
​
```

Risk

Likelihood:

  • Medium — while not always immediately exploitable, violating the CEI pattern increases the risk of reentrancy or state inconsistency if external calls are made before internal state updates. Exploitation depends on whether the called contracts are untrusted.

Impact:

The impact is likely low but its impoartant that the CEI patterns is properly executed in this code base for proper functionality of the protocol.
​

Proof of Concept

<details>
<summary>Code</summary>
​
```diff
​
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
​
- require(countryIndex < teams.length, "Invalid country index");
​
if (_setWinner) {
revert WinnerAlreadySet();
}
​
- winnerCountryId = countryIndex;
- winner = teams[countryIndex];
​
- _setWinner = true;
​
_getWinnerShares();
​
_setFinallizedVaultBalance();
​
emit WinnerSet (winner);
return winner;
​
}
​
```

Recommended Mitigation

- remove this code
+ add this code
​
Follow the CEI pattern for proper functionality
​
```diff
​
+function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
+ if (block.timestamp <= eventEndDate) {
+ revert eventNotEnded();
+ }
​
+ if (_setWinner) {
+ revert WinnerAlreadySet();
+ }
+ require(countryIndex < teams.length, "Invalid country index");
​
+ _setWinner = true;
+ winnerCountryId = countryIndex;
+ winner = teams[countryIndex];
+ _getWinnerShares();
​
+ _setFinallizedVaultBalance();
​
+ emit WinnerSet (winner);
+ return winner;
​
}
​
```
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!