Summary
An attacker can sell less than divided pieces, thus leading to buyers holding tokens that they cannot use to claim.
Vulnerability Details
There is no check in the sellErc20() function that verifies that the amount of tokens listed (to be sold) are indeed the total amount of dividend token pieces.
Impact
A user can buy token pieces from the marketplace under the assumption that the listed tokens are all the dividend token pieces. When that user goes to claim the NFT, they get reverted with an insufficient amount error.
PoC:
Alice divides her NFT into 5 token pieces
Alice lists only 5 pieces to be sold
Bob buys all 5 pieces under the assumption that they are all the dividend token pieces for the NFT
Bob goes to claim, but gets reverted
Alice lists the remaining 5 pieces
Unsuspecting Clara buys all 5 of the newly listed tokens under the assumption that they are all the dividend token pieces for the NFT
Clara goes to claim, but also gets reverted
Bob and Clara are left with token pieces which they can not claim
If there is no way to know who has the remaining pieces, these tokens become totally useless
Add the following test to the test file:
function test_can_sell_less_than_pieces_divided() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), 1e18);
tokenDivider.sellErc20(address(erc721Mock), 1e18, 1e18);
vm.stopPrank();
assertEq(tokenDivider.getBalanceOf(USER, address(erc20Mock)), 1e18);
assertEq(erc20Mock.balanceOf(address(tokenDivider)), 1e18);
vm.prank(USER2);
tokenDivider.buyOrder{value: (2e18)}(0, USER);
vm.startPrank(USER2);
erc20Mock.approve(address(tokenDivider), 1e18);
vm.expectRevert();
tokenDivider.claimNft(address(erc721Mock));
vm.stopPrank();
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), 1e18);
tokenDivider.sellErc20(address(erc721Mock), 1e18, 1e18);
vm.stopPrank();
address patrick = makeAddr("unaware_buyer");
vm.deal(patrick, 2e18);
vm.prank(patrick);
tokenDivider.buyOrder{value: 2e18}(0, USER);
vm.startPrank(patrick);
erc20Mock.approve(address(tokenDivider), 1e18);
vm.expectRevert();
tokenDivider.claimNft(address(erc721Mock));
vm.stopPrank();
}
Tools Used
Recommendations
function sellErc20(address nftPegged, uint256 price, uint256 amount) external {
if (nftPegged == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
if (amount == 0) {
revert TokenDivider__AmountCantBeZero();
}
ERC20Info memory tokenInfo = nftToErc20Info[nftPegged];
if (balances[msg.sender][tokenInfo.erc20Address] < amount) {
revert TokenDivider__InsuficientBalance();
}
+ // Ensure the amount matches the total ERC20 tokens minted for the NFT
+ require(
+ amount == erc20ToMintedAmount[tokenInfo.erc20Address],
+ "TokenDivider: Amount must match total divided pieces"
+ );
balances[msg.sender][tokenInfo.erc20Address] -= amount;
s_userToSellOrders[msg.sender].push(
SellOrder({seller: msg.sender, erc20Address: tokenInfo.erc20Address, price: price, amount: amount})
);
emit OrderPublished(amount, msg.sender, nftPegged);
IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, address(this), amount);
}
Here is a test to confirm this mitigation:
function test_cannot_sell_less_than_pieces_divided() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
vm.expectRevert("TokenDivider: Amount must match total divided pieces");
tokenDivider.sellErc20(address(erc721Mock), 1e18, 1e18);
vm.stopPrank();
}
