Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Tokens Can Be Permanently Trapped Due To Lack of Cancel Sell Order Functionality

guzbyte

Summary

The TokenDivider contract lacks a mechanism to cancel sell orders. Once a user creates a sell order using sellErc20(), their tokens are transferred to the contract and can become permanently trapped if no one buys the order. The only way to recover the tokens is through a successful sale, which may never happen.

Vulnerability Details

When a user creates a sell order:

  1. Their tokens are deducted from their balance

  2. Tokens are transferred to the contract

  3. No mechanism exists to cancel the order and recover tokens

  4. Tokens remain locked until someone purchases the order

This creates a scenario where user assets can become permanently inaccessible if their sell order is not attractive to buyers or if the market conditions change.

Impact

  • HIGH: Potential permanent loss of user assets

  • Users cannot recover their tokens if their sell order doesn't attract buyers

  • Forces users to keep unfavorable sell orders active

  • No way to respond to changing market conditions

Proof of Concept

function testTokensCanBePermanentlyTrapped() public {
// Setup
vm.startPrank(USER);
erc721Mock.approve(address(tokenDivider), TOKEN_ID);
tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT);
// Get the ERC20 token address
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
erc20Mock.approve(address(tokenDivider), AMOUNT);
// Create sell order
uint256 highPrice = 1000 ether; // Unreasonably high price that no one will buy
tokenDivider.sellErc20(address(erc721Mock), highPrice, AMOUNT);
// Verify tokens are trapped:
// 1. Tokens are in contract
assertEq(erc20Mock.balanceOf(address(tokenDivider)), AMOUNT);
// 2. User's balance is zero
assertEq(tokenDivider.getBalanceOf(USER, address(erc20Mock)), 0);
// 3. User cannot transfer tokens (they don't have them)
vm.expectRevert();
tokenDivider.transferErcTokens(address(erc721Mock), USER2, AMOUNT);
// 4. User cannot claim NFT (don't have tokens)
vm.expectRevert(TokenDivider.TokenDivider__NotEnoughErc20Balance.selector);
tokenDivider.claimNft(address(erc721Mock));
// 5. No function exists to cancel order and recover tokens
// Tokens remain trapped unless someone buys at the high price
vm.stopPrank();
}

Tools Used

Manual review

Foundry for POC

Recommendations

Implement a cancelSellOrder function:

​
event OrderCancelled(
address indexed seller,
address indexed erc20Address,
uint256 amount
);
​
function cancelSellOrder(uint256 orderIndex) external {
SellOrder[] storage orders = s_userToSellOrders[msg.sender];
require(orderIndex < orders.length, "Invalid order index");
SellOrder memory orderToCancel = orders[orderIndex];
require(orderToCancel.seller == msg.sender, "Not order owner");
// Return tokens to seller's balance
balances[msg.sender][orderToCancel.erc20Address] += orderToCancel.amount;
// Remove order
orders[orderIndex] = orders[orders.length - 1];
orders.pop();
// Return tokens to seller
IERC20(orderToCancel.erc20Address).transfer(msg.sender, orderToCancel.amount);
emit OrderCancelled(msg.sender, orderToCancel.erc20Address, orderToCancel.amount);
}

Consider adding a time limit on orders after which they can be automatically cancelled.

Updates

Lead Judging Commences

juan_pedro_ventu Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Sell orders cant be canceled

Appeal created

juan_pedro_ventu Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Sell orders cant be canceled

Support

FAQs

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