The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Transferring SmartVault NFT in onERC721Received function on buyer contract will revert

matejdb

Summary

When calling the mint() function, from a smart contract that is used for minting new vaults, on Smart Vault Manager contract and trying to transfer the NFT in the onERC721Received(address, address, uint256, bytes calldata) to a different address (lets say the buyer address) the call will revert.

Vulnerability Details

Transferring of Smart Vault NFT in onERC721Received(address, address, uint256, bytes calldata) is not possible because the Smart Vault is not yet deployed. Therefore, the _afterTokenTransfer(address, address, uint256, uint256) hook can not execute correctly because it can not retrieve non-existing vault address.

Buyer contract used for testing vulnerability:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.17;
import "@openzeppelin/contracts-upgradeable/token/ERC721/IERC721ReceiverUpgradeable.sol";
​
import "./SmartVaultManagerV5.sol";
​
contract Buyer {
SmartVaultManagerV5 public smartVaultManager;
​
address public buyer;
​
constructor(address _smartVaultManager, address _buyer) {
smartVaultManager = SmartVaultManagerV5(_smartVaultManager);
buyer = _buyer;
}
​
function mint() external {
smartVaultManager.mint();
}
​
function onERC721Received(
address /* operator */,
address /* from */,
uint256 tokenId,
bytes calldata /* data */
) external returns (bytes4) {
smartVaultManager.safeTransferFrom(address(this), buyer, tokenId);
return this.onERC721Received.selector;
}
}
​

Unit test written in smartVaultManager.js -> it passes

it('Transferring Smart Vault NFT when minting with Buyer contract should fail', async () => {
const BuyerFactory = await ethers.getContractFactory('Buyer');
const BuyerContract = await BuyerFactory.deploy(VaultManager.address, user.address);
​
const mint = BuyerContract.connect(user).mint();
await expect(mint).to.be.reverted;
})

Impact

Core functionality is severed here, therefore the HIGH issue tag. Impact is self-evident. User that uses a smart contract to use the protocol can not use it properly.

Tools Used

Manual review

Recommendations

Move _safeMint(msg.sender, tokenId); line from SmartVaultManager below the smartVaultIndex.addVaultAddress(tokenId, payable(vault)); in mint() function.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

matejdb Submitter
over 1 year ago
hrishibhat Lead Judge
over 1 year ago
matejdb Submitter
over 1 year ago
hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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