Summary
The contract allows multiple NFTs from the same collection to be fractionalized into fungible ERC20 tokens. However, it only uses the NFT collection address (not the token ID) as a mapping key. This design causes subsequent fractionalizations to overwrite previous ones, resulting in a Denial of Service for earlier fractionalized NFTs and potential loss of access to tokens.
Vulnerability Details
The core issue lies in how the contract’s nftToErc20Info
mapping is designed. The mapping uses the NFT collection’s address nftAddress
as the key and associates it with the most recently fractionalized token’s information. This approach creates a mapping collision when multiple token IDs from the same collection are fractionalized.
When divideNft(address nftAddress, uint256 tokenId, uint256 amount)
is called, the contract updates the mapping:
nftToErc20Info[nftAddress] = ERC20Info({ erc20Address: erc20, tokenId: tokenId });
If an NFT with a new token ID from the same collection is subsequently fractionalized, it overwrites the existing entry in the mapping for that collection’s address. As a result, the data for the previously fractionalized NFT is lost. When the original owner of the first fractionalized NFT attempts to reclaim it by providing the correct amount of the ERC20 tokens, the contract no longer references their token ID’s data.
This prevents the owner from accessing their fractional tokens or reclaiming their NFT, creating a Denial of Service and the potential for permanent token loss.
Impact
-
Denial of Service for Legitimate NFT Owners:
-
Potential NFT Token Loss:
Tools Used
POC
pragma solidity ^0.8.28;
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {ERC20Mock} from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import {Test, console} from "forge-std/Test.sol";
import {DeployTokenDivider} from "script/DeployTokenDivider.s.sol";
import {TokenDivider} from "src/TokenDivider.sol";
* @dev A simple ERC721 mock contract for testing.
*/
contract ERC721Mock is ERC721, Ownable {
uint256 private _tokenIdCounter;
constructor() ERC721("MockToken", "MTK") Ownable(msg.sender) {}
function mint(address to) public onlyOwner {
_safeMint(to, _tokenIdCounter);
_tokenIdCounter++;
}
}
* @dev This test contract demonstrates how fractionalizing two different NFTs
* from the same ERC721 collection by different legitimate users leads to overwrites,
* causing a Denial of Service for the first user's NFT.
*/
contract DDoSNFT is Test {
DeployTokenDivider deployer;
TokenDivider tokenDivider;
ERC721Mock erc721Mock;
address userA = makeAddr("userA");
address userB = makeAddr("userB");
function setUp() public {
deployer = new DeployTokenDivider();
tokenDivider = deployer.run();
erc721Mock = new ERC721Mock();
erc721Mock.mint(userA);
erc721Mock.mint(userB);
vm.deal(userA, 10 ether);
vm.deal(userB, 10 ether);
console.log("Setup complete. userA has tokenId=0, userB has tokenId=1.");
}
* @notice Demonstrates how overwriting the mapping with a second fractionalization
* breaks the first user's ability to `claimNft`.
*/
function test_divideNft_issue() public {
vm.startPrank(userA);
erc721Mock.approve(address(tokenDivider), 0);
tokenDivider.divideNft(address(erc721Mock), 0, 1_000_000 ether);
vm.stopPrank();
address userA_Erc20 = tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address;
console.log("userA fraction token:", userA_Erc20);
vm.startPrank(userB);
erc721Mock.approve(address(tokenDivider), 1);
tokenDivider.divideNft(address(erc721Mock), 1, 1_000_000 ether);
vm.stopPrank();
address userB_Erc20 = tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address;
console.log("userB fraction token:", userB_Erc20);
vm.startPrank(userA);
console.log("Attempting to claim NFT #0, but it's overwritten by tokenId=1 data...");
vm.expectRevert();
tokenDivider.claimNft(address(erc721Mock));
vm.stopPrank();
}
}
% forge test --match-contract DDoSNFT -vv
[⠊] Compiling...
[⠒] Compiling 1 files with Solc 0.8.28
[⠑] Solc 0.8.28 finished in 542.56ms
Compiler run successful!
Ran 1 test for test/unit/AccessLostToNft.t.sol:DDoSNFT
[PASS] test_divideNft_issue() (gas: 2200161)
Logs:
Setup complete. userA has tokenId=0, userB has tokenId=1.
userA fraction token: 0xFEfC6BAF87cF3684058D62Da40Ff3A795946Ab06
userB fraction token: 0x2a9e8fa175F45b235efDdD97d2727741EF4Eee63
Attempting to claim NFT #0, but it's overwritten by tokenId=1 data...
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.72ms (1.31ms CPU time)
Ran 1 test suite in 135.71ms (7.72ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommendations
Recommendations
Delete the ERC20Info
struct and the erc20ToNft
mapping in order to use the a more granular per tokenId mapping:
mapping(address collection => mapping(uint256 tokenId => address erc20TokenOfCollection)) collectionToTokenIdToErc20Address;
Update the divideNft
function to use the new mapping:
function divideNft(address nftAddress,uint256 tokenId,uint256 amount) external onlyNftOwner(nftAddress, tokenId)
{
if (amount == 0) revert TokenDivider__AmountCantBeZero();
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, "");
if (IERC721(nftAddress).ownerOf(tokenId) == msg.sender) revert TokenDivider__NftTransferFailed();
collectionToTokenIdToErc20Address[nftAddress][tokenId] = erc20;
balances[msg.sender][erc20] = amount;
erc20ToMintedAmount[erc20] = amount;
emit NftDivided(nftAddress, tokenId, amount, erc20);
bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
if (!transferSuccess) revert TokenDivider__TransferFailed();
}
Update the claimNft
function to use the new tokenId granular strategy:
function claimNft(address nftAddress, uint256 tokenId) external {
address erc20Address = collectionToTokenIdToErc20Address[nftAddress][tokenId];
if (balances[msg.sender][erc20Address] !== erc20ToMintedAmount[erc20Address]) {
revert TokenDivider__NotEnoughErc20Balance();
}
ERC20ToGenerateNftFraccion(erc20Address).burnFrom(msg.sender, erc20ToMintedAmount[erc20Address]);
balances[msg.sender][erc20Address] = 0;
erc20ToMintedAmount[erc20Address] = 0;
emit NftClaimed(nftAddress, tokenId, erc20Address);
IERC721(nftAddress).safeTransferFrom(address(this), msg.sender, tokenId);
}
Update the transferErcTokens
function to use the new tokenId granular strategy:
function transferErcTokens(address nftAddress, uint256 tokenId, address to, uint256 amount) external {
if (amount == 0) revert TokenDivider__AmountCantBeZero();
address erc20Address = collectionToTokenIdToErc20Address[nftAddress][tokenId];
if (balances[msg.sender][erc20Address] < amount) revert TokenDivider__NotEnoughErc20Balance();
balances[msg.sender][erc20Address] -= amount;
balances[to][erc20Address] += amount;
emit TokensTransfered(amount, erc20Address);
IERC20(erc20Address).transferFrom(msg.sender, to, amount);
Update the sellErc20
function to use the new tokenId granular strategy:
function sellErc20(address nftPegged, uint256 tokenId, uint256 price, uint256 amount) external {
if (amount == 0) revert TokenDivider__AmountCantBeZero();
address erc20Address = collectionToTokenIdToErc20Address[nftPegged][tokenId];
if (balances[msg.sender][erc20Address] < amount) revert TokenDivider__InsuficientBalance();
balances[msg.sender][erc20Address] -= amount;
s_userToSellOrders[msg.sender].push(
SellOrder({ seller: msg.sender, erc20Address: erc20Address, price: price, amount: amount })
);
emit OrderPublished(amount, msg.sender, nftPegged);
IERC20(erc20Address).transferFrom(msg.sender, address(this), amount);
}
Update the getErc20InfoFromNft
function to use the new tokenId granular strategy:
function getErc20InfoFromNft(address nft, uint256 tokenId) public view returns (address) {
return collectionToTokenIdToErc20Address[nft][tokenId];
}
Update the events to use the new tokenId granular strategy, note also the indexed parameters are updated:
event NftDivided(address indexed nftAddress, uint256 indexed tokenId, uint256 amountErc20Minted, address erc20Minted);
event NftClaimed(address indexed nftAddress, uint256 indexed tokenId, address erc20Address);