First Flight #7: Horse Store
First Flight #7
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong logic in `HorseStore::IS_HAPPY_HORSE`

honour

Description: HorseStore::IS_HAPPY_HORSE contains HORSE_HAPPY_IF_FED_WITHIN < timestamp - horseFedTimestamp & timestamp == horseFedTimestamp conditional checks rather than HORSE_HAPPY_IF_FED_WITHIN >= timestamp - horseFedTimestamp check .This means that the HorseStore::IS_HAPPY_HORSE function will always return false if it's not called in the same transaction as HorseStore::FEED_HORSE (or with the same timestamp) or if it's called within the HORSE_HAPPY_IF_FED_WITHIN(24 hr) period.

Impact: High, If horse X has been fed within the past 24 hours, horse X must be happy. is an invariant in the system

Proof of Code:

Code
function test_FeedingDoesNotMakeHappyHorse() public {
uint256 horseId = horseStore.totalSupply();
vm.prank(user);
horseStore.mintHorse();
horseStore.feedHorse(horseId);
skip(60 * 60); //1 hr
assertEq(horseStore.isHappyHorse(horseId), false);
}

Recommedation:

#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]
+ swap2 swap1 sub // [timestamp - horseFedTimestamp, HORSE_HAPPY_IF_FED_WITHIN > timestamp - horseFedTimestamp]
+ [HORSE_HAPPY_IF_FED_WITHIN_CONST] // [HORSE_HAPPY_IF_FED_WITHIN, timestamp - horseFedTimestamp, HORSE_HAPPY_IF_FED_WITHIN >
+ timestamp - horseFedTimestamp]
+ eq or // [HORSE_HAPPY_IF_FED_WITHIN >= timestamp - horseFedTimestamp]
​
start_return_true jumpi // [ ]
- eq // [timestamp == horseFedTimestamp]
+ 0x0
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.