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
}