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

The timestamp condition in the `setWinner` function is not precise enough.

minos

The timestamp condition in the setWinner function is not precise enough.

Description

  • When the admin executes the setWinner function to announce the "winning national team's name" under the premise that the event has ended.

  • However, there is a slight inaccuracy in determining whether the "event has ended."

function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
@> if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
// ... original code
}

  • In common programming paradigms, we generally consider that block.timestamp >= eventEndDate indicates the event has ended.

  • But in this code, block.timestamp <= eventEndDate is used to represent "the event has not ended yet," which is clearly inaccurate.

Risk

Likelihood:

  • In the block where the timestamp exactly equals eventEndDate, it will definitely occur.

Impact:

  • Inconsistent with the event's advertised end time.

  • The admin cannot set the winner immediately at the exact moment the event ends; they must wait until eventEndDate + 1 second or later to execute, causing unnecessary delays.

  • Users expect to see the result immediately after the event ends, but they may need to wait for one or more additional block confirmations, potentially leading to doubts about the project team's execution capability.

Proof of Concept

  • Add the following function in test/BriVaultTest.t.sol, then run forge test --mt test__setWinner_inEventEndDateBlock -vv

function test__setWinner_inEventEndDateBlock() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.warp(eventEndDate);
vm.expectRevert(abi.encodeWithSelector(BriVault.eventNotEnded.selector));
string memory winner = briVault.setWinner(2);
vm.stopPrank();
}

  • The console will print:

Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] test__setWinner_inEventEndDateBlock() (gas: 1345020)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.65ms (768.40µs CPU time)

Recommended Mitigation

function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
- if (block.timestamp <= eventEndDate) {
+ if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
// ... original code
}
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!