Description
The value produced by the lt instruction corresponds to the "horse NOT happy" variable since it becomes true when the time difference between the current timestamp and the last fed timestamp is bigger than the HORSE_HAPPY_IF_FED_WITHIN interval.
HORSE_HAPPY_IF_FED_WITHIN < timestamp - horseFedTimestamp
However, this failure is not detected if the isHappyHorse function is called with the same timestamp as the feedHorse function, in that case, the result of timestamp == horseFedTimestamp will be true, placed on the stack and this value will be returned instead.
Impact
The IS_HAPPY_HORSE function returns incorrect results.
Proof of concept
Call isHappyHorse at a different timestamp than the feedHorse function, but still within the HORSE_HAPPY_IF_FED_WITHIN interval.
function testFeedingMakesHappyHorse() public {
uint256 horseId = horseStore.totalSupply();
uint256 timeStampFed = 10000000;
uint256 waitInterval = horseStore.HORSE_HAPPY_IF_FED_WITHIN() - 10;
vm.warp(timeStampFed);
vm.roll(timeStampFed);
vm.prank(user);
horseStore.mintHorse();
horseStore.feedHorse(horseId);
vm.warp(timeStampFed + waitInterval);
vm.roll(timeStampFed + waitInterval);
assertEq(horseStore.isHappyHorse(horseId), true);
}
Recommended mitigation
Negate the value produced by the lt operation using iszero and return it directly.
0x04 calldataload // [horseId]
LOAD_ELEMENT(0x00) // [horseFedTimestamp]
timestamp // [timestamp, horseFedTimestamp]
- dup2 dup2 // [timestamp, horseFedTimestamp, timestamp, horseFedTimestamp]
sub // [timestamp - horseFedTimestamp]
[HORSE_HAPPY_IF_FED_WITHIN_CONST] // [HORSE_HAPPY_IF_FED_WITHIN, timestamp - horseFedTimestamp]
lt // [HORSE_HAPPY_IF_FED_WITHIN < timestamp - horseFedTimestamp]
+ iszero // [HORSE_HAPPY_IF_FED_WITHIN >= timestamp - horseFedTimestamp]
+ 0x00 mstore
+ 0x20 0x00 return
- start_return_true jumpi //
- eq
- start_return
- jump
- start_return_true:
- 0x01
- start_return:
- // Store value in memory.
- 0x00 mstore
- // Return value
- 0x20 0x00 return