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

Redundant and Ineffective Timestamp Check in withdraw

hark017

Description

  • The withdraw function should only allow claims after the winner is set, which already enforces post-event-end timing via the winnerSet modifier.

It redundantly checks block.timestamp < eventEndDate despite setWinner requiring > eventEndDate, creating misleading logic without additional protection.

function withdraw() external winnerSet { // @> Modifier ensures post-end
if (block.timestamp < eventEndDate) { // @> Redundant; always false here
revert eventNotEnded();
}
// ...
}

Risk

Likelihood:

  • During the window after eventEndDate but before owner calls setWinner.

In code reviews or audits where the redundancy confuses sequencing assumptions.

Impact:

  • Delays withdrawals unnecessarily if owner is slow to set winner, locking winner liquidity.

Misleads developers integrating the contract, potentially causing incorrect off-chain timing logic.

Proof of Concept

// Assume eventEndDate passed, but winner not set
function testRedundantCheck() public {
// block.timestamp > eventEndDate
vm.prank(winnerUser);
// Call withdraw: passes winnerSet? No, reverts winnerNotSet
// But if modifier removed, the inner check would still pass since timestamp > end
// Redundancy hides that true block is modifier
}

Recommended Mitigation

function withdraw() external winnerSet {
- if (block.timestamp < eventEndDate) {
- revert eventNotEnded();
- }
// Rely on winnerSet for timing
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(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!