Pieces Protocol

First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: high
Invalid

Zero Price in SellOrders Enables Free Token Acquisition, Risking Seller Assets and Protocol Fees

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:

  • leads to direct financial loss for sellers.

  • allows malicious actors to exploit the system to drain assets from sellers and bypass protocol fees.

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:

    • Loss of assets for sellers.

    • Loss of protocol fees.

  • sellErc20 Function

function sellErc20(address nftPegged, uint256 price,uint256 amount) external {
if(nftPegged == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
if( amount == 0) { //<====== Here
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

  • buyOrder Function

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;//<======== Calculating protocol fee
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);
// Transfer The Ether
(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.

  • Setup: add the following code on TokenDividerTest.t.sol and run forge test --mt testSellWithZeroPrice -vvvv and forge test --mt testBuyOrderWithZeroPrice -vvvv

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); // get price of order at index 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);
// Record initial balance
uint256 userBalanceBefore = address(USER).balance;
vm.startPrank(USER);
// Approve token divider to spend USER's tokens
erc20Mock.approve(address(tokenDivider), AMOUNT);
// Sell ERC20 tokens at a price of 0
tokenDivider.sellErc20(address(erc721Mock), 0, AMOUNT); // Selling AMOUNT tokens at zero price
vm.stopPrank();
vm.startPrank(USER2);
// User2 attempts to buy the NFT with 0 Ether
tokenDivider.buyOrder{value: 0}(0, USER); // Buying with 0 Ether
assertEq(erc20Mock.balanceOf(USER2), AMOUNT, "USER2 should have received the ERC20 tokens");
// Check token divider balance
assertEq(erc20Mock.balanceOf(address(tokenDivider)), 0, "Token divider should not hold any ERC20 tokens after the sale");
// Verify balances of ether for USER and contract owner remain unchanged
assertEq(address(USER).balance, userBalanceBefore, "USER's ether balance should remain unchanged");
}

Impact

  • Loss of asset: Sellers may lose their tokens without receiving any payment.

  • Loss of protocol fee: The protocol does not collect fees if transactions occur at a zero price.

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();
}
Updates

Lead Judging Commences

fishy Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.