Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Invalid

`HorseStore.huff::IS_HAPPY_HORSE` contains erroneous implementation which causes it to return incorrect answers

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
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.