Summary
Both the Solidity and Huff versions of the HorseStore smart contract contain functions (feedHorse in Solidity and FEED_HORSE() macro in Huff) that allow users to feed horses represented by NFTs. These functions update a mapping (horseIdToFedTimeStamp) that records the last time each horse was fed. However, there is no check in place to verify that a horse with the given ID has actually been minted and exists before allowing the feed operation to proceed.
Vulnerability Details
- HorseStore.sol
function feedHorse(uint256 horseId) external {
@> horseIdToFedTimeStamp[horseId] = block.timestamp;
}
- HorseStore.huff
#define macro FEED_HORSE() = takes (0) returns (0) {
timestamp
@> 0x04 calldataload
STORE_ELEMENT(0x00)
0x11 timestamp mod
endFeed jumpi
revert
endFeed:
stop
}
Impact
While this issue may not directly result in financial loss or token theft, it can lead to a polluted contract state where non-existent horses have feeding timestamps. This could complicate the contract's operation, lead to incorrect assumptions about the number of horses, and potentially be exploited in unforeseen ways in combination with other contract functions.
function testFeedingNotExistingHorse() public {
uint256 horseId = horseStore.totalSupply();
console2.log("The horse id totalSupply: %s", horseId);
vm.warp(10);
vm.roll(10);
vm.prank(user);
horseStore.mintHorse();
uint256 lastFedTimeStamp = block.timestamp;
uint256 horseToFedId = horseId + 1;
console2.log("The horse id to fed: %s", horseToFedId);
horseStore.feedHorse(horseToFedId);
assertEq(horseStore.horseIdToFedTimeStamp(horseToFedId), lastFedTimeStamp);
console2.log("The horse id feeded is:", horseToFedId);
}
Running 1 test for test/HorseStoreSolidity.t.sol:HorseStoreSolidity
[PASS] testFeedingNotExistingHorse() (gas: 119934)
Logs:
The horse id totalSupply: 0
The horse id to fed: 1
The horse id feeded is: 1
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 15.13ms
Running 1 test for test/HorseStoreHuff.t.sol:HorseStoreHuff
[PASS] testFeedingNotExistingHorse() (gas: 90700)
Logs:
The horse id totalSupply: 0
The horse id to fed: 1
The horse id feeded is: 1
Tools Used
Manual review.
Recommendations
To address this vulnerability, the contract should be modified to include a check that confirms the existence of a horse before allowing the feedHorse function to update its last fed timestamp.
function feedHorse(uint256 horseId) external {
+ require(horseId < totalSupply(), "HorseStore: invalid horseId");
horseIdToFedTimeStamp[horseId] = block.timestamp;
}
#define macro FEED_HORSE() = takes (0) returns (0) {
// Load the horseId from calldata
0x04 calldataload // [horseId]
// Check if the horse exists by comparing horseId with totalSupply
+ [TOTAL_SUPPLY] sload // [totalSupply, horseId]
+ dup2 lt // [horseId < totalSupply, totalSupply, horseId]
+ iszero horse_does_not_exist jumpi // If horseId >= totalSupply, jump to error handling
// Update the horse's fed timestamp
timestamp // [timestamp, totalSupply, horseId]
STORE_ELEMENT(0x00) // []
// End execution
stop
// Error handling for non-existent horse
horse_does_not_exist:
// Revert the transaction
revert(0x00, 0x00)
}
