Summary
Incorrect calculations in the file HorseStore.huff in the function IS_HAPPY_HORSE lead to incorrect output
Vulnerability Details
In function
https://github.com/Cyfrin/2024-01-horse-store/blob/8255173c9340c12501881c9ecdd4175ff7350f5d/src/HorseStore.huff#L93C1-L115C2
#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
start_return_true:
0x01
start_return:
0x00 mstore
0x20 0x00 return
}
At line
https://github.com/Cyfrin/2024-01-horse-store/blob/8255173c9340c12501881c9ecdd4175ff7350f5d/src/HorseStore.huff#L100C1
uses lt instead of gt which leads to the horse not being happy for 24 hours after feeding and being happy after that time, which is the opposite of the description of a projet
POC
Add this test to HorseStoreHuff.t.sol and run via forge test --mt testHappyAfter -vv to see it success.
function testHappyAfter() public {
uint256 horseId = horseStore.totalSupply();
vm.warp(10);
vm.prank(user);
horseStore.mintHorse();
horseStore.feedHorse(horseId);
assertEq(horseStore.isHappyHorse(horseId), true);
for (uint delay = 1; delay <= 24 * 60 * 60; delay++) {
vm.warp(10 + delay);
assertEq(horseStore.isHappyHorse(horseId), false);
}
for (uint delay = 24 * 60 * 60 + 1; delay <= 10 * 24 * 60 * 60; delay++) {
vm.warp(10 + delay);
assertEq(horseStore.isHappyHorse(horseId), true);
}
}
Output
Running 1 test for test/HorseStoreHuff.t.sol:HorseStoreHuff
[PASS] testHappyAfter() (gas: 2697862881)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.91s
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Opposite of the correct function output for the function IS_HAPPY_HORSE, excluding the case when block.timestamp = horseFedTimestamp
Tools Used
Foundry, manual inspection.
Recommendations
Make the following changes:
https://github.com/Cyfrin/2024-01-horse-store/blob/8255173c9340c12501881c9ecdd4175ff7350f5d/src/HorseStore.huff#L93C1-L115C2
#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
}
