BriVault

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

Logic Flaw and Redundancy on _setFinallizedVaultBalance()

();

ernal Logic Check _setFinallizedVaultBalance() Uses Incorrect Time Boundary

Description

  • The internal function _setFinallizedVaultBalance() contains an inverted time check that verifies the event has not started (block.timestamp <= eventStartDate), which is intended to run only after the event has already ended. This check is logically inconsistent and rendered entirely redundant by the stricter time validation in its external caller (setWinner()).


  • The _setFinallizedVaultBalance() function is called exclusively by setWinner(). The check in setWinner() ensures the event has ended (block.timestamp > eventEndDate). Since eventStartDate is always less than eventEndDate, if the setWinner() check passes, the check inside the internal function is logically inconsistent and unnecessary.

    function _setFinallizedVaultBalance() internal returns (uint256) {
    // @> START OF REDUNDANT/INCORRECT CHECK @>
    if (block.timestamp <= eventStartDate) {
    revert eventNotStarted(); // INCORRECT REVERT: Should be eventNotEnded
    }
    // @> END OF REDUNDANT/INCORRECT CHECK @>
    return finalizedVaultAsset = IERC20(asset()).balanceOf(address(this));
    }


Risk

Likelihood: Very Low (Non-Exploitable)

Reason 1: The _setFinallizedVaultBalance() function is only callable via setWinner(), which requires block.timestamp > eventEndDate. The eventEndDate is strictly later than eventStartDate. Therefore, any successful call to setWinner() will automatically bypass the condition block.timestamp <= eventStartDate inside the internal function, meaning the revert is never triggered.



Impact:

Minimal

Code Quality: Introduces misleading logic and confusion for future developers maintaining the codebase.

Gas: Increases bytecode size and consumes unnecessary gas for a redundant state check on every execution path of setWinner()

Proof of Concept

Since the bug is non-exploitable, the proof focuses on demonstrating the redundancy by observing the call trace.

The trace shows that when setWinner is called (which requires the time to be well past eventEndDate), the check in _setFinallizedVaultBalance is passed without issue, proving its irrelevance.

Explanation

1. The test fast-forwards time well past eventEndDate.

2. setWinner() is called and succeeds, proving block.timestamp > eventEndDate.

3. The trace confirms the call enters _setFinallizedVaultBalance().

4. The internal function proceeds without reverting, confirming the check against eventStartDate is ineffective.

\

Since the bug is non-exploitable, the proof focuses on demonstrating the **redundancy** by observing the call trace.

Recommended Mitigation

The internal check is entirely redundant and should be removed.

// BriVault.sol
function _setFinallizedVaultBalance() internal returns (uint256) {
- if (block.timestamp <= eventStartDate) {
- revert eventNotStarted();
- }
return finalizedVaultAsset = IERC20(asset()).balanceOf(address(this));
}
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!