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

Invalid check in _setFinallizedVaultBalance

eigaku6326

Root + Impact

Description

  • The normal behavior of the _setFinallizedVaultBalance() function is to set and return the final vault asset balance, which is used for the reward distribution after the winner is determined.

  • In this function, there is an unnecessary time check: block.timestamp <= eventStartDate. This check will never be triggered in the actual execution process because the only place where this function is called, the setWinner() function, already has stricter time constraints.

// Root cause in the codebase with @> marks to highlight the relevant section
*/
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
@> if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
​
function _setFinallizedVaultBalance () internal returns (uint256) {
@> if (block.timestamp <= eventStartDate) {
revert eventNotStarted();
}

Risk

Likelihood:

  • Reason 1 Extremely low - Because in the calling path, the setWinner() function has already verified that block.timestamp > eventEndDate, and the event end time is necessarily greater than or equal to the start time, so this check will never be triggered.

  • Reason 2 This check has no practical effect in the contract and is completely redundant code.

Impact:

  • Impact 1 Code redundancy - It adds unnecessary complexity to the code and reduces its maintainability.

  • Impact 2 Potential confusion - For auditors or developers, they might misunderstand the purpose and execution path of this code.

Proof of Concept

// Call chain analysis:
// 1. The _setFinallizedVaultBalance() function is only called within the setWinner() function.
// 2. The setWinner() function requires that block.timestamp > eventEndDate.
// 3. Since eventEndDate >= eventStartDate (the end time of the event is later than the start time),
// 4. Therefore, block.timestamp > eventStartDate is necessarily true.
// 5. Thus, the check in _setFinallizedVaultBalance() will never be triggered.

Recommended Mitigation

function _setFinallizedVaultBalance () internal returns (uint256) {
- if (block.timestamp <= eventStartDate) {
- revert eventNotStarted();
- }
​
return finalizedVaultAsset = IERC20(asset()).balanceOf(address(this));
}
Updates

Appeal created

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