Description

The marketplace logic (publish order via sellErc20, buy order via buyOrder) has a few design and security considerations:

1. Out-of-Bounds orderIndex: buyOrder does not check if orderIndex is < s_userToSellOrders[seller].length. An invalid orderIndex could revert with an out-of-bounds array access.

2. Partial Order or Cancels: Orders appear to be “all-or-nothing.” Once published, you either buy the entire amount in the order, or you do not. There is no direct function to cancel an existing order besides manually purchasing your own order.

3. Fees: The code splits a fee from order.price / 100, then a portion of that fee is taken from the buyer. This is an unusual approach (the code demands the buyer send order.price + sellerFee). If incorrectly set, it could create confusion or allow corner cases with zero or extremely large prices.

Impact

• User Reverts: If orderIndex is invalid, the entire buy transaction reverts with no user-friendly error.

• Lack of Cancellation: A seller who changes their mind or sets the wrong price can only remove an order by re-buying it themselves or bridging a custom solution.

• Fee Confusion: Unexpected or untested edge cases (e.g., zero or extremely large prices) might cause weird fee calculations or revert paths.

Recommendation

• Validate orderIndex prior to array access:

  require(orderIndex < s_userToSellOrders[seller].length, "Invalid order index");

• Consider implementing a cancel function so a seller can remove their active order.

• Carefully document or refine how fees are calculated and confirm all edge cases.