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 {
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);
horseStore.mintHorse();
console2.log("nft's minted", horseStore.totalSupply());
horseStore.feedHorse(2);
uint256 time = horseStore.horseIdToFedTimeStamp(2);
console2.log("fed horse id 2 at time, which does not exist:", time);
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 -
add a supply check
+ error InvalidHorseId();
function feedHorse(uint256 horseId) external {
+ if(totalSupply() < 1 && hourseId > totalSupply() - 1){
+ revert InvalidHorseId(); }
horseIdToFedTimeStamp[horseId] = block.timestamp;
}
Check owner address for id
function feedHorse(uint256 horseId) external {
+ if(ownerOf(hourseId) == address(0)){
+ revert InvalidHorseId(); }
horseIdToFedTimeStamp[horseId] = block.timestamp;
}