Description
The buyOrder function collects the protocol fee and seller's payment via two separate safeTransferFrom calls. If the first call (for the protocol fee) succeeds and the second call fails (for the seller’s payment), the contract may end up in an inconsistent state, and the buyer’s funds could be partially deducted without completing the order.
function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
if (order.seller == address(0)) revert OrderNotFound();
if (!order.isActive) revert OrderNotActive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired();
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
@> iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
@> iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
totalFees += protocolFee;
emit OrderFilled(_orderId, msg.sender, order.seller);
}
Impact:
This creates a broken atomicity guarantee: the buyer may lose USDC to fees even though the order does not complete. It could also be used for griefing attacks.
Proof of Concept
Recommended Mitigation
Collect the full USDC payment in one step, then split the payment internally:
function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// Validation checks
if (order.seller == address(0)) revert OrderNotFound();
if (!order.isActive) revert OrderNotActive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired();
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
- iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
- iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
- IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
+ iUSDC.safeTransferFrom(msg.sender, address(this), order.priceInUSDC);
+ iUSDC.safeTransfer(order.seller, sellerReceives);
totalFees += protocolFee;
emit OrderFilled(_orderId, msg.sender, order.seller);
}
```
