Pieces Protocol

Summary

The TokenDivider contract uses an array-based indexing (orderIndex) to track multiple sell orders. After a sell order is fulfilled, it swaps and pops from the array, causing subsequent orders to move and change indexes. A malicious seller can exploit this by front-running or self-buying an order—reordering the array and causing a legitimate buyer to purchase a less valuable or worthless order at the same index. Essentially, the buyer’s intended order index no longer corresponds to the desired sell order once the array is modified.

Vulnerability Details

1. Array-Based Order Tracking
   Each seller maintains an array s_userToSellOrders[seller]. New sell orders get appended. When an order is purchased, the code removes it by copying the last array element into the purchased index and then calling .pop(). This approach inadvertently reorders the array if you do not purchase the last element.

2. Front-Running Attack

   • Suppose an order at index=0 is selling 10 fraction tokens for 1 ETH.

   • A second order at index=1 might be selling only 1 fraction token (or 1 wei of a fraction token) for the same 1 ETH.

   • If a buyer tries to buy the “good” order at index=0, the seller can front-run the transaction (or in the same block) and buy it themselves (or otherwise remove it).

   • The contract’s array deletion swaps the worthless order into index=0.

   • Now, when the legitimate buyer’s transaction executes, they still pass orderIndex=0, but they end up buying the worthless order (1 fraction token for 1 ETH) instead of the intended 10 tokens for 1 ETH.

3. Unstable Index References
   Because the contract references orders by an ephemeral array index rather than a stable unique ID, the index can change at any time if any other user buys an order, or if the malicious seller intentionally manipulates the array. This leads to unexpected behavior for other buyers relying on a “trusted” index.

Proof Of Concept

Impact

• Buyer Pays Full Price for a Minimal (Worthless) Order: A legitimate buyer sends enough Ether for a large fraction token bundle but receives a smaller bundle (or effectively worthless tokens) because the array index was swapped.

• Financial Loss / Scamming: Attackers can “bait” buyers into thinking they are buying a high-value order. By reordering the array at the last moment, the buyer instead purchases an inferior order.

• Market Confusion: Multiple orders from the same seller can be reordered, making it impossible for buyers to rely on array indices. The marketplace becomes unsafe and unpredictable.

Recommendations

1. Use Unique Order Identifiers

   • Instead of referencing orders by array index, generate a unique orderId (like an incrementing counter or a hash) to store in a mapping. For example:

     mapping(address user => mapping(uint256 orderId => SellOrder orders)) sellerToOrder;

     This approach ensures stable references even if the underlying data structure changes.

   • Use this orderId strategy to store new orders in the sellerToOrder mapping, then return the orderId created to the caller, so modify the function to:

     function sellErc20(address nftPegged, uint256 tokenId, uint256 price, uint256 amount) external returns (uint256 orderId) {

     ​

     ... same code as before ...

     ​

     orderId = nextOrderId++;

     sellerToOrder[msg.sender][orderId] = SellOrder({

     seller: msg.sender,

     erc20Address: erc20Address,

     price: price,

     amount: amount

     });

     ​

     // Below old code

     // s_userToSellOrders[msg.sender].push(

     // SellOrder({ seller: msg.sender, erc20Address: erc20Address, price: price, amount: amount })

     // );

     ​

     ... same code as before ...
     function buyOrder(uint256 orderId, address seller) external payable {

     SellOrder memory order = sellerToOrder[seller][orderId];

     // ensure the integrity of the order

     if (order.seller != seller) revert TokenDivider__Unauthorized();

     ​

     ... same code as before ...

     ​

     // Remove the order from the user's orders

     delete sellerToOrder[seller][orderId];

     ​

     // Below old code

     // s_userToSellOrders[seller][orderIndex] = s_userToSellOrders[seller][s_userToSellOrders[seller].length - 1];

     // s_userToSellOrders[seller].pop();

     ​

     ... same code as before ...

     if you followed [H-5] Missing ability to delete open sell orders, update the cancel order too:

     function cancelOrder(uint256 orderId) external {

     SellOrder memory order = sellerToOrder[msg.sender][orderId];

     ​

     ​

     ... same code as before ...

     ​

     // Remove the order from the user's orders

     delete sellerToOrder[msg.sender][orderId];

     // Below old code

     // s_userToSellOrders[seller][orderIndex] = s_userToSellOrders[seller][s_userToSellOrders[seller].length - 1];

     // s_userToSellOrders[seller].pop();

   • Use orderId to reference orders in internal functions and events. For example:

     emit OrderPublished(order.amount, msg.sender, order.erc20Address, orderId);

     emit OrderSelled(msg.sender, order.price, orderId);

   • Update getOrderPrice to use the orderId instead of the orderIndex:

     function getOrderPrice(address seller, uint256 orderId) public view returns (uint256 price) {

     price = sellerToOrder[seller][orderId].price;

     }