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

Lack of `onERC721Received` in `safeTransferFrom` in the huff version

0xlouistsai

Summary

The safeMInt function in Solidity version will verify whether the address is an EOA or an contract address, if it is the latter one, the mint operation should check the return value of onERC721Received in receiving contract to safety.

Vulnerability Details

Add a mock contract that implement the onERC721Received function.

contract MockReceiver {
​
HorseStore internal horseStore;
​
constructor(HorseStore _horseStore) payable {
horseStore = _horseStore;
}
​
function mintHorse() external {
horseStore.mintHorse();
}
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external returns (bytes4) {
return bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"));
}
}

Add the following test in the Base_Test, run forge test --match-test testMintHorseContract

function testMintHorseContract() public {
vm.prank(user);
MockReceiver receiver = new MockReceiver(horseStore);
receiver.mintHorse();
​
assertEq(horseStore.balanceOf(address(receiver)), 1);
}

It will both succeed in solidity version and huff version. However, if we remove the onERC721Received function in the receiving contract, the anticipated behavior is to revert the transaction since the receiving contract does not follow the IERC721Receiver pattern.

Impact

Not implementing the IERC721Receiver validation is dangerous since the receiving contract might not be able to withdraw or do operation to the received NFT, in this case, the NFT will be locked in the contract forever, and the difference between the two version is also an issue.

Tools Used

Foundry

Recommendations

Implement the validation of onERC721Received in the mint operation and check whether contract address has the ability to handle the NFT.

Updates

Lead Judging Commences

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

Components of ERC721 not properly (or at all) implemented in HUFF

Support

FAQs

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