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