Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Invalid

`HorseStore:: feedHorse` lack of input validation will cause waste of gas

Summary

Lack of input validation in feedHorse function

Vulnerability Details

feedHorse function take horseId as input and execute the function.

/*
* @param horseId the id of the horse to feed
* @notice allows anyone to feed anyone else's horse.
*/
function feedHorse(uint256 horseId) external {
/// @audit missing input validation
horseIdToFedTimeStamp[horseId] = block.timestamp;
}

Issue lies within it's execution, as it does not check if input horseId has been minted or not. User can loose gas fees unnecessarily if they input invalid horseId.

#POC
add a following test in existing test suite.

function testFeedNonExistenceHorse () public {
vm.startPrank(user);
/// user mint a horse (supply 1, id 0)
horseStore.mintHorse();
console2.log("nft's minted", horseStore.totalSupply());
/// let's feed horse 2 which doesn't exist
horseStore.feedHorse(2);
uint256 time = horseStore.horseIdToFedTimeStamp(2);
console2.log("fed horse id 2 at time, which does not exist:", time);
/// verify id 2 not exist
vm.expectRevert();
horseStore.ownerOf(2);
}

then run following command in terminal forge test --mt testFeedNonExistenceHorse -vv, you'll see following results

Running 1 test for test/HorseStoreHuff.t.sol:HorseStoreHuff
[PASS] testFeedNonExistenceHorse() (gas: 121859)
Logs:
nft's minted 1
fed horse id 2 at time, which does not exist: 1705165260
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.58ms

Impact

Unnecessary gas fees wastage if non existence nftId is put as input.

Tools Used

Manual Review

Recommendations

Here are few recommendations -

  1. add a supply check

+ error InvalidHorseId();
function feedHorse(uint256 horseId) external {
+ if(totalSupply() < 1 && hourseId > totalSupply() - 1){
+ revert InvalidHorseId(); }
horseIdToFedTimeStamp[horseId] = block.timestamp;
}
  1. Check owner address for id

function feedHorse(uint256 horseId) external {
+ if(ownerOf(hourseId) == address(0)){
+ revert InvalidHorseId(); }
horseIdToFedTimeStamp[horseId] = block.timestamp;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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