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

It is possible to feed a horse with an invalid token ID, i.e., an ID that does not correspond to any minted horse NFT

stefanlatinovic

Summary

It is possible to feed a horse whose token has not been minted yet or is burned.

Vulnerability Details

It is possible to feed a horse with an ID that has not yet been minted and subsequently mint a horse with the same ID. This can lead to incorrect results when verifying the horse's happiness, as the verification process may indicate that the horse is happy even when it shouldn't be.

Impact

Feeding a non-minted horse may cause false happiness verification results post-minting.

Proof of Concept (PoC)

Add the next test in Base_Test.t.sol.

function test_FeedingNotMintedHorseIsAllowed(uint256 horseId, uint256 feedAt) public {
vm.warp(feedAt);
horseStore.feedHorse(horseId); // feeding a horse that is not minted is allowed
​
assertEq(horseStore.horseIdToFedTimeStamp(horseId), feedAt);
}

Run a test with forge test --mt test_FeedingNotMintedHorseIsAllowed.

Tools Used

  • Foundry

Recommendations

To properly feed a horse, it's important to verify the validity of the token ID.

Recommended changes in HorseStore.sol::feedHorse() function:

function feedHorse(uint256 horseId) external {
+ _requireOwned(horseId); // reverts if the token with `horseId` has not been minted yet, or if it has been burned
horseIdToFedTimeStamp[horseId] = block.timestamp;
}

Add the next test to HorseStoreSolidity.t.sol.

import {IERC721Errors} from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
​
function test_FeedingHorseBeforeMintingItIsNotAllowed(address randomOwner) public {
vm.assume(randomOwner != address(0));
vm.assume(!_isContract(randomOwner));
​
uint256 horseId = horseStore.totalSupply();
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, horseId));
horseStore.feedHorse(horseId);
}

Run a test with forge test --mt test_FeedingHorseBeforeMintingItIsNotAllowed.

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

#define macro FEED_HORSE() = takes (0) returns (0) {
timestamp // [timestamp]
0x04 calldataload // [horseId, timestamp]
+ dup1 // [horseId, horseId, timestamp]
+ // revert if owner is zero address/not minted
+ [OWNER_LOCATION] LOAD_ELEMENT_FROM_KEYS(0x00) // [owner, horseId, timestamp]
+ dup1 continue jumpi
+ NOT_MINTED(0x00)
+ continue:
+ dup3 dup3 // [horseId, timestamp, owner, horseId, timestamp]
+ STORE_ELEMENT(0x00) // [owner, horseId, timestamp]
​
// End execution
0x11 timestamp mod
endFeed jumpi
revert
endFeed:
stop
}

Add the next test to HorseStoreHuff.t.sol.

function test_FeedingHorseBeforeMintingItIsNotAllowed(address randomOwner) public {
vm.assume(randomOwner != address(0));
vm.assume(!_isContract(randomOwner));
​
uint256 horseId = horseStore.totalSupply();
vm.expectRevert("NOT_MINTED");
horseStore.feedHorse(horseId);
}

Run a test with forge test --mt test_FeedingHorseBeforeMintingItIsNotAllowed.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Nonexistent horses can be fed

Support

FAQs

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