Summary
The IS_HAPPY_HORSE macro is incorrectly comparing the elapsed time since feeding a horse and the the time limit for the horse to be happy, so it returns that the horse is unhappy.
Vulnerability Details
This opcode is comparing if the time limit is lower than the time elapsed since feeding, which is incorrect.
#define macro IS_HAPPY_HORSE() = takes (0) returns (0) {
0x04 calldataload
LOAD_ELEMENT(0x00)
timestamp
dup2 dup2
sub
[HORSE_HAPPY_IF_FED_WITHIN_CONST]
@> lt
start_return_true jumpi
eq
start_return
jump
Impact
Horses that were fed are returned as unhappy, breaking the rule that a horse is happy when it is fed.
Tools Used
Foundry, Manual review
Proof of Concept
This is a test that feeds a horse, warps time and expects it to be unhappy.
function testHorseIsUnhappyAfterFeeding() public {
vm.warp(horseStore.HORSE_HAPPY_IF_FED_WITHIN());
vm.prank(user);
horseStore.mintHorse();
vm.warp(block.timestamp + 3600);
horseStore.feedHorse(0);
vm.warp(block.timestamp + 3600);
bool isHappyHorse = horseStore.isHappyHorse(0);
assertEq(isHappyHorse, false);
}
Run Huff test
forge test --mc HorseStoreHuff --mt testFeedOnInvalidTimestamp
Test passes, confirming that the horse is unhappy after feeding.
Running 1 test for test/HorseStoreHuff.t.sol:HorseStoreHuff
[PASS] testHorseIsUnhappyAfterFeeding() (gas: 107151)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.50s
Recommended Mitigation
Correct the conditionals.
0x04 calldataload // [horseId]
LOAD_ELEMENT(0x00) // [horseFedTimestamp]
timestamp // [timestamp, horseFedTimestamp]
dup2 dup2 // [timestamp, horseFedTimestamp, timestamp, horseFedTimestamp]
sub // [timestamp - horseFedTimestamp, timestamp, horseFedTimestamp]
[HORSE_HAPPY_IF_FED_WITHIN_CONST] // [HORSE_HAPPY_IF_FED_WITHIN, timestamp - horseFedTimestamp, timestamp, horseFedTimestamp]
- lt // [HORSE_HAPPY_IF_FED_WITHIN < timestamp - horseFedTimestamp, timestamp, horseFedTimestamp]
+ gt // [HORSE_HAPPY_IF_FED_WITHIN > timestamp - horseFedTimestamp, timestamp, horseFedTimestamp]
start_return_true jumpi // [timestamp, horseFedTimestamp]
eq // [timestamp == horseFedTimestamp]
start_return
jump
