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

`IS_HAPPY_HORSE` macro in huff returns false even though the horse is happy

Summary

The horse must be happy if fed within the past 24 hours, but inside the huff contract isHappyHorse returns false even though horse is actually happy due to wrong implementation inside IS_HAPPY_HORSE macro.

Vulnerability Details

The vulnerability lies inside the IS_HAPPY_HORSE macro due to wrong checks at line 100.
It says that if HORSE_HAPPY_IF_FED_WITHIN < timestamp - horseFedTimestamp then the horse is happy but instead it should be
HORSE_HAPPY_IF_FED_WITHIN > timestamp - horseFedTimestamp

As timestamp - horseFedTimestamp is the seconds ago the horse was last fed, so true should be return if it is less than HORSE_HAPPY_IF_FED_WITHIN.

Impact

Returns false even if horse is happy

PoC

Add the test in the file - test/HorseStoreHuff.t.sol

Run the test:

forge test --mt test_isHappyHorse_ReturnsFalse_EvenIfHorseIsHappy -vv
function test_isHappyHorse_ReturnsFalse_EvenIfHorseIsHappy() public {
uint256 horseId = horseStore.totalSupply();
vm.prank(user);
horseStore.mintHorse();
vm.warp(horseStore.HORSE_HAPPY_IF_FED_WITHIN());
horseStore.feedHorse(horseId);
vm.warp(block.timestamp + 1);
console.log(horseStore.isHappyHorse(horseId));
}

Tools Used

Manual Review

Recommendations

Return true if timestamp - horseFedTimestamp < HORSE_HAPPY_IF_FED_WITHIN

#define macro IS_HAPPY_HORSE() = takes (0) returns (0) {
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
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
Validated
Assigned finding tags:

Wrong comparison in IS_HAPPY_HORSE()

Support

FAQs

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