Description The divideNft function is designed to split an NFT into ERC20 tokens. Each time this function is called, it creates a new ERC20 protocol, meaning that one NFT collection can correspond to multiple ERC20 tokens.
To keep track of the relationship between an NFT and its ERC20 tokens, we use a mapping: mapping(address nft => ERC20Info) nftToErc20Info. The ERC20Info struct contains the ERC20 address and the token ID.
Whenever divideNft is called, the mapping is updated with a new value. If we call divideNft multiple times using the same NFT address from the same user (enforced by the onlyNftOwner modifier) but with different token IDs, the mapping will overwrite the existing entry in nftToErc20Info with a new ERC20Info object for that NFT address. As a result, the previous information about the ERC20 tokens is lost.
At the end, each NFT (ERC721) and its tokenId has a different ERC20 address. In our case, the ERC20 tokens have been lost within the protocol, and there is no way to recover them. As a result, the NFT being deposited into the protocol is now frozen and effectively stolen, putting us in a difficult situation.
Impact When information about ERC20 tokens is lost in the contract, users can no longer interact with those lost tokens within the protocol. Additionally, the NFT (ERC721) is also frozen inside the protocol. This situation represents a high-severity vulnerability.
Proof of Concepts
First, we mint NFT to a user.
Then the user calls the divideNft function with the NFT address that we've minted and tokenId of 0
Then we mint one more NFT to the same user - This step also can happen with Step 1.
The user calls again divideNft function with tokenId of 1 (for new NFT of collection)
Now mapping has overwritten the ERC20Info about the first nft that we divided
Place the following code in TokenDividerTest.t.sol file:
function testNftDivideSameCollectionDiffTokenId() public {
erc721Mock.mint(USER);
uint256 FIRST_AMOUNT = 6e18;
uint256 SECOND_AMOUNT = 1e18;
erc721Mock.mint(USER);
vm.startPrank(USER);
erc721Mock.approve(address(tokenDivider), 0);
tokenDivider.divideNft(address(erc721Mock), 0, FIRST_AMOUNT);
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
console.log("ERC20 address: ", address(erc20Mock));
console.log("User balance after first nft divided: ", tokenDivider.getBalanceOf(USER, address(erc20Mock)));
erc721Mock.approve(address(tokenDivider), 1);
tokenDivider.divideNft(address(erc721Mock), 1, SECOND_AMOUNT);
vm.stopPrank();
ERC20Mock erc20Mock2 = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
console.log("ERC20 address: ", address(erc20Mock2));
console.log("User balance after second nft divided: ", tokenDivider.getBalanceOf(USER, address(erc20Mock2)));
vm.startPrank(USER);
uint256 orderPrice = SECOND_AMOUNT;
vm.expectRevert(TokenDivider.TokenDivider__InsuficientBalance.selector);
tokenDivider.sellErc20(address(erc721Mock), orderPrice, FIRST_AMOUNT);
erc20Mock2.approve(address(tokenDivider), SECOND_AMOUNT);
tokenDivider.claimNft(address(erc721Mock));
vm.stopPrank();
assertEq(erc721Mock.ownerOf(TOKEN_ID), address(tokenDivider));
}
And run the following command: forge test --mt testNftDivideSameCollectionDiffTokenId -vvvv
Recommended Mitigation Consider updating the existing functionality by adding an additional mapping in the current mapping. Now, ERC20Info will not be limited to one ERC20Info for one NFT Collection; instead, there will be a separate ERC20Info for every single token ID.
contract TokenDivider is IERC721Receiver, Ownable {
.
.
.
struct ERC20Info {
address erc20Address; // e ERC20 token address
uint256 tokenId; // e collection id
}
.
.
mapping(address user => mapping(address erc20Address => uint256 amount)) balances;
- mapping(address nft => ERC20Info) nftToErc20Info;
+ mapping(address nft => mapping (uint256 tokenId => ERC20Info)) nftToErc20Info;
mapping(address user => SellOrder[] orders) s_userToSellOrders;
mapping(address erc20 => address nft) erc20ToNft;
mapping(address erc20 => uint256 totalErc20Minted) erc20ToMintedAmount;
function divideNft(address nftAddress, uint256 tokenId, uint256 amount)
external
onlyNftOwner(nftAddress, tokenId)
onlyNftOwner(nftAddress, tokenId)
{
.
.
.
if (IERC721(nftAddress).ownerOf(tokenId) == msg.sender) revert TokenDivider__NftTransferFailed();
balances[msg.sender][erc20] = amount;
- nftToErc20Info[nftAddress] = ERC20Info({erc20Address: erc20, tokenId: tokenId});
+ nftToErc20Info[nftAddress][tokenId] = ERC20Info({erc20Address: erc20, tokenId: tokenId});
erc20ToMintedAmount[erc20] = amount; // amount of erc20 minted for the nft
erc20ToNft[erc20] = nftAddress;
emit NftDivided(nftAddress, amount, erc20);
bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
if (!transferSuccess) {
revert TokenDivider__TransferFailed();
}
}
function claimNft(
address nftAddress,
+ uint256 tokenId
) external {
if (nftAddress == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
- ERC20Info storage tokenInfo = nftToErc20Info[nftAddress];
+ ERC20Info storage tokenInfo = nftToErc20Info[nftAddress][tokenId];
.
.
.
}
function transferErcTokens(
address nftAddress,
+ uint256 tokenId,
address to,
uint256 amount
) external {
.
.
.
- ERC20Info memory tokenInfo = nftToErc20Info[nftAddress];
+ ERC20Info memory tokenInfo = nftToErc20Info[nftAddress][tokenId];
.
.
.
}
function sellErc20(
address nftPegged,
+ uint256 tokenId,
uint256 price,
uint256 amount
) external {
.
.
.
- ERC20Info memory tokenInfo = nftToErc20Info[nftPegged];
+ ERC20Info memory tokenInfo = nftToErc20Info[nftPegged][tokenId];
.
.
.
}
/**
* Getters
*/
function getErc20InfoFromNft(
address nft
+ ,uint256 tokenId
) public view returns (ERC20Info memory) {
- return nftToErc20Info[nft];
+ return nftToErc20Info[nft][tokenId];
}
