[M-1] `FestivalPass::attendPerformance` relies on `block.timestamp` which can be manipulated by miners
Description
-
The `FestivalPass::attendPerformance` function uses a 1-hour cooldown to prevent users from attending performances.
-
However, this function relies on the `lastCheckIn` mapping, which maps a user to a `uint256` timestamp (`block.timestamp`) that can be manipulated by malicious miners.
function attendPerformance(uint256 performanceId) external {
require(isPerformanceActive(performanceId), "Performance is not active");
require(hasPass(msg.sender), "Must own a pass");
require(!hasAttended[performanceId][msg.sender], "Already attended this performance");
@> require(block.timestamp >= lastCheckIn[msg.sender] + COOLDOWN, "Cooldown period not met");
hasAttended[performanceId][msg.sender] = true;
@> lastCheckIn[msg.sender] = block.timestamp;
uint256 multiplier = getMultiplier(msg.sender);
BeatToken(beatToken).mint(msg.sender, performances[performanceId].baseReward * multiplier);
emit Attended(msg.sender, performanceId, performances[performanceId].baseReward * multiplier);
}
Risk
Likelihood:
Impact:
Proof of Concept
1. Organizer creates performances
1. Performance 1 is 1 hour long and starts in an hour
2. Performance 2 is 1 hour long and starts 30 min after the first
2. User buys a pass
3. User attends performance 1
4. Malicious miner goes back 30 minutes
5. User wants to attend performance 2 but it is not active, so it reverts
function testBlockTimestampManipulationPreventsAttendingPerformance() public {
vm.startPrank(organizer);
uint256 startTime1 = block.timestamp + 1 hours;
uint256 startTime2 = startTime1 + 30 minutes;
uint256 perfId1 = festivalPass.createPerformance(startTime1, 1 hours, 1000e18);
uint256 perfId2 = festivalPass.createPerformance(startTime2, 1 hours, 1000e18);
vm.stopPrank();
vm.prank(user1);
festivalPass.buyPass{value: GENERAL_PRICE}(1);
vm.warp(startTime1 + 30 minutes);
vm.prank(user1);
festivalPass.attendPerformance(perfId1);
vm.warp(startTime1 - 30 minutes);
vm.prank(user1);
vm.expectRevert("Performance is not active");
festivalPass.attendPerformance(perfId2);
}
Recommended Mitigation
Since `block.timestamp` is manipulatable, consider using `block.number` instead.
- uint256 constant COOLDOWN = 1 hours;
+ uint256 constant COOLDOWN = 300; // number of blocks in an hour (1 block every ~12s)
// ...
function attendPerformance(uint256 performanceId) external {
require(isPerformanceActive(performanceId), "Performance is not active");
require(hasPass(msg.sender), "Must own a pass");
require(!hasAttended[performanceId][msg.sender], "Already attended this performance");
- require(block.timestamp >= lastCheckIn[msg.sender] + COOLDOWN, "Cooldown period not met");
+ require(block.number >= lastCheckIn[msg.sender] + COOLDOWN, "Cooldown period not met");
hasAttended[performanceId][msg.sender] = true;
- lastCheckIn[msg.sender] = block.timestamp;
+ lastCheckIn[msg.sender] = block.number;
uint256 multiplier = getMultiplier(msg.sender);
BeatToken(beatToken).mint(msg.sender, performances[performanceId].baseReward * multiplier);
emit Attended(msg.sender, performanceId, performances[performanceId].baseReward * multiplier);
}
