Eggstravaganza
First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

No ERC721 Receiver Lead to Breaks safeTransferFrom() compatibility

pratamaajip01

Summary

The EggVault contract does not implement the onERC721Received function, which is required for receiving ERC721 tokens via safeTransferFrom(). As a result, any attempt to transfer an NFT to the vault using safeTransferFrom() will fail. This breaks compatibility with the ERC721 standard and may lead to unexpected behavior in integrations.

Vulnerability Details

When transferring an NFT using safeTransferFrom(), the ERC721 standard requires the recipient contract to implement the onERC721Received function. This function ensures that the contract can accept ERC721 tokens and prevents accidental transfers to contracts that do not support them.

However, the EggVault contract does not implement IERC721Receiver, causing safeTransferFrom() calls to fail.

Impact

  • Breaks ERC721 compatibility, preventing integrations that rely on safeTransferFrom().

  • Causes unexpected transaction failures, potentially leading to lost user funds or broken game mechanics.

  • Makes deposits unreliable, as NFTs must be transferred using transferFrom() instead, which does not check whether the recipient is a valid contract.

Tools Used

  • Manual Review

  • Foundry

PoC

// @audit - vault doesn't implement onERC721Received
function testSafeTransferFailsWithoutReceiver() public {
vm.startPrank(address(game));
nft.mintEgg(address(game), 2);
​
vm.expectRevert(); // should fail because vault doesn't implement onERC721Received
nft.safeTransferFrom(address(game), address(vault), 2);
vm.stopPrank();
}

Recommendations

The vault contract should implement the IERC721Receiver interface and correctly handle incoming NFT transfers.

Modify the EggVault contract to ensure that any contract receiving an NFT implements onERC721Received.

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.23;
​
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; // Import IERC721Receiver
import "./EggstravaganzaNFT.sol";
​
-contract EggVault is Ownable {
+contract EggVault is Ownable, IERC721Receiver { // Implement IERC721Receiver
/// @notice Reference to the EggstravaganzaNFT contract.
EggstravaganzaNFT public eggNFT;
​
/// @notice Mapping to track deposited eggs (tokenId => deposited status).
mapping(uint256 => bool) public storedEggs;
/// @notice Mapping to record who deposited each egg.
mapping(uint256 => address) public eggDepositors;
​
event EggDeposited(address indexed depositor, uint256 tokenId);
event EggWithdrawn(address indexed withdrawer, uint256 tokenId);
​
constructor()Ownable(msg.sender){}
​
/// @notice Set the NFT contract address.
function setEggNFT(address _eggNFTAddress) external onlyOwner {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}
​
/// @notice Records the deposit of an egg (NFT).
/// The NFT must already have been transferred to the vault.
function depositEgg(uint256 tokenId, address depositor) public {
require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred to vault");
require(!storedEggs[tokenId], "Egg already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = depositor;
emit EggDeposited(depositor, tokenId);
}
/// @notice Allows the depositor to withdraw their egg from the vault.
function withdrawEgg(uint256 tokenId) public {
require(storedEggs[tokenId], "Egg not in vault");
require(eggDepositors[tokenId] == msg.sender, "Not the original depositor");
storedEggs[tokenId] = false;
delete eggDepositors[tokenId];
​
eggNFT.transferFrom(address(this), msg.sender, tokenId);
emit EggWithdrawn(msg.sender, tokenId);
}
​
/// @notice Checks if a specific egg is currently deposited.
function isEggDeposited(uint256 tokenId) public view returns (bool) {
return storedEggs[tokenId];
}
​
+ /// @notice Required to accept ERC721 transfers using safeTransferFrom.
+ function onERC721Received(
+ address operator,
+ address from,
+ uint256 tokenId,
+ bytes calldata data
+ ) external pure override returns (bytes4) {
+ return IERC721Receiver.onERC721Received.selector;
+ }
}
​
​
Updates

Lead Judging Commences

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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