Description
The contract uses timestamp comparisons to enforce timing restrictions for various operations. However, two functions use incorrect comparison operators that wrongly allow or prevent operations at boundary timestamps.
The `setWinner()` function checks `if (block.timestamp <= eventEndDate)` which prevents setting the winner at exactly `eventEndDate`, when it should be allowed, because the event has ended. The `joinEvent()` function checks `if (block.timestamp > eventStartDate)` which allows joining at exactly `eventStartDate`, when it should be prevented once the event has started.
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
if (block.timestamp <= eventEndDate) {
revert eventNotEnded();
}
}
function joinEvent(uint256 countryId) public {
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
}
Risk
**Likelihood**:
* Block timestamps can match exact event boundaries
* Users can time transactions to join when event has already started
* Owner can't set winner at exact event end time
**Impact**:
* Users may join event at the exact start time when they shouldn't be able to
* Owner can't set winner at exact end date, even though the event is over
* Potential race conditions and timing edge cases
* Inconsistent behavior with contract's intended timing logic
Proof of Concept
The PoC test below shows how a user would be able to join at the time when the event starts and the owner would not be able to set the winner at the time when the event has ended.
function test_TimestampEdgeCases() public {
vm.prank(owner);
briVault.setCountry(countries);
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
vm.warp(eventStartDate);
briVault.joinEvent(10);
vm.stopPrank();
vm.warp(eventEndDate);
vm.prank(owner);
vm.expectRevert(abi.encodeWithSignature("eventNotEnded()"));
briVault.setWinner(10);
}
Recommended Mitigation
Switch the lower or equal to lower on setWinner and the greater to greater or equal on joinEvent.
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
- if (block.timestamp <= eventEndDate) {
+ if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
// ... rest of function ...
}
function joinEvent(uint256 countryId) public {
// ... validation code ...
- if (block.timestamp > eventStartDate) {
+ if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
// ... rest of function ...
}
