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

Checking if a horse is happy after being fed in the past 24 hours returns incorrect result, violating the invariant

stefanlatinovic

Summary

A horse is marked as not happy even if it has been fed within the past 24 hours.

Vulnerability Details

The current implementation of the HorseStore.huff::IS_HAPPY_HORSE() function won't provide an accurate result as it returns false even if the horse has been fed within the past 24 hours.

Impact

One of the contract's invariants has been violated because the check for the horse's happiness is resulting in a false negative.

Proof of Concept (PoC)

Add the next test in HorseStoreHuff.t.sol.

function test_HorseIsNotHappyIfFedWithinPast24Hours(uint256 horseId, uint256 checkAt) public {
uint256 fedAt = horseStore.HORSE_HAPPY_IF_FED_WITHIN();
checkAt = bound(checkAt, fedAt + 1 seconds, fedAt + horseStore.HORSE_HAPPY_IF_FED_WITHIN() - 1 seconds); // it has been less than 24 hours since the last feeding time
vm.warp(fedAt);
horseStore.feedHorse(horseId);
vm.warp(checkAt);
assertEq(horseStore.isHappyHorse(horseId), false); // horse seems unhappy despite being fed less than 24 hours ago
}

Run a test with forge test --mt test_HorseIsNotHappyIfFedWithinPast24Hours.

Tools Used

  • Foundry

Recommendations

Recommended changes to HorseStore.huff::IS_HAPPY_HORSE() function:

#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
}

Add the next test in HorseStoreHuff.t.sol.

function test_HorseIsHappyIfFedWithinPast24Hours(uint256 horseId, uint256 checkAt) public {
uint256 fedAt = horseStore.HORSE_HAPPY_IF_FED_WITHIN();
checkAt = bound(checkAt, fedAt + 1 seconds, fedAt + horseStore.HORSE_HAPPY_IF_FED_WITHIN() - 1 seconds); // it has been less than 24 hours since the last feeding time
vm.warp(fedAt);
horseStore.feedHorse(horseId);
vm.warp(checkAt);
assertEq(horseStore.isHappyHorse(horseId), true);
}

Run a test with forge test --mt test_HorseIsHappyIfFedWithinPast24Hours.

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.