Summary
https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L221
The report identifies a critical issue where a seller can input a price of zero when listing assets in the sellErc20 function. This oversight allows buyers to acquire tokens for free, resulting in potential financial losses for sellers and the protocol. Additionally, this affects the buyOrder function, where the fee logic is bypassed, and protocol fees are not collected.
Vulnerability Details
Severity
High Risk:
Root cause
https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L221
The vulnerability arises from the lack of validation on the price parameter in the sellErc20 function. Although there is a check to ensure that the amount is not zero, there is no corresponding check for the price. This oversight could lead to scenarios where:
-
A seller lists their asset at a price of zero.
-
Buyers can purchase these assets without any payment, leading to:
-
sellErc20 Function
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();
}
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);
}
This issue also affects the protocol in the BuyOrder function as fee could be un-accounted for: https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L274-L276
function buyOrder(uint256 orderIndex, address seller) external payable {
if(seller == address(0)) {
revert TokenDivider__InvalidSeller();
}
SellOrder memory order = s_userToSellOrders[seller][orderIndex];
if(msg.value < order.price) {
revert TokenDivider__IncorrectEtherAmount();
}
uint256 fee = order.price / 100;
uint256 sellerFee = fee / 2;
if(msg.value < order.price + sellerFee) {
revert TokenDivider__InsuficientEtherForFees();
}
balances[msg.sender][order.erc20Address] += order.amount;
s_userToSellOrders[seller][orderIndex] = s_userToSellOrders[seller][s_userToSellOrders[seller].length - 1];
s_userToSellOrders[seller].pop();
emit OrderSelled(msg.sender, order.price);
(bool success, ) = payable(order.seller).call{value: (order.price - sellerFee)}("");
if(!success) {
revert TokenDivider__TransferFailed();
}
(bool taxSuccess, ) = payable(owner()).call{value: fee}("");
if(!taxSuccess) {
revert TokenDivider__TransferFailed();
}
IERC20(order.erc20Address).transfer(msg.sender, order.amount);
}
On Line 274, the fee is calculated to be order.price / 100 which means if the order.price is 0, the Seller gets no payment, owner gets no fees, while the Buyer gets tokens for free. Which means a loss for both the protocol (owner) and the Seller.
POC
There are two tests on the PoC below.
One Test if zero price is accepted by the sellErc20 function and the other tests the result if an order has a zero price.
Test 1: Attempt to sell with zero price
function testSellWithZeroPrice() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.sellErc20(address(erc721Mock), 0, AMOUNT);
vm.stopPrank();
uint256 price = tokenDivider.getOrderPrice(USER,0);
assertEq(price,0,"Erc20 Price should not be set to zero");
// Check that USER's balance of ERC20 is now 0
assertEq(tokenDivider.getBalanceOf(USER, address(erc20Mock)), 0, "User should have 0 ERC20 tokens after selling");
}
Test 2: Attempt to buy a zero-priced order
function testBuyOrderWithZeroPrice() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
uint256 userBalanceBefore = address(USER).balance;
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.sellErc20(address(erc721Mock), 0, AMOUNT);
vm.stopPrank();
vm.startPrank(USER2);
tokenDivider.buyOrder{value: 0}(0, USER);
assertEq(erc20Mock.balanceOf(USER2), AMOUNT, "USER2 should have received the ERC20 tokens");
assertEq(erc20Mock.balanceOf(address(tokenDivider)), 0, "Token divider should not hold any ERC20 tokens after the sale");
assertEq(address(USER).balance, userBalanceBefore, "USER's ether balance should remain unchanged");
}
Impact
Tools Used
Manual Review & Foundry
Recommendations
Add Validation to Prevent price to be Zero in sellErc20:
Ensure that price cannot be 0 when the order is created or executed. Add a check like this in the sellErc20 function:
if (price == 0) {
revert TokenDivider__InvalidOrderPrice();
}
Add validation to check for order.price not to be zero in buyOrder
Ensure that order.price cannot be 0 when the order is created or executed. Add a check like this in the buyOrder function:
if (order.price == 0) {
revert TokenDivider__InvalidOrderPrice();
}