OrderBook

First Flight #43
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Impact: medium
Likelihood: high
Invalid

MEV Vulnerability (Front-running Risk on Order Filling)

Understood. I will ensure 'Impact' and 'Likelihood' are bolded as per the requested format.

Here is the revised report:



Finding: MEV Vulnerability (Front-running Risk on Order Filling)



Description


The OrderBook contract, as an on-chain trading mechanism, is susceptible to Maximal Extractable Value (MEV) attacks, specifically front-running. While createSellOrder and amendSellOrder have low direct front-running risk, the buyOrder function is vulnerable to front-running, allowing malicious actors to exploit profitable order listings at the expense of legitimate users.


Risk


Root Cause: The buyOrder function lacks a mechanism for the buyer to specify a maximum acceptable price (slippage control). This means a user's transaction to fill a favorably priced order can be observed in the mempool and exploited.

Solidity

function buyOrder(uint256 _orderId) public {
Order storage order = orders[_orderId];
// ... (validation checks)
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);
// Funds are transferred based on order.priceInUSDC without buyer's price limit.
totalFees += protocolFee;
emit OrderFilled(_orderId, msg.sender, order.seller);
}

Likelihood: High. Front-running bots actively monitor mempools for profitable opportunities on transparent blockchains. An attractive order (e.g., significantly below market price) would almost certainly be front-run.

Impact: Medium.

  • Poor User Experience: Legitimate buyers attempting to fill favorable orders will consistently lose out to bots, leading to frustration and disengagement.

  • Economic Unfairness: Profitable opportunities are captured by sophisticated actors rather than the intended participants, diminishing the fairness of the trading platform.

  • Not a direct fund loss from the contract: Funds are not stolen from the contract itself, nor are the initial sellers directly harmed. The impact is primarily on the buyers missing out on a good deal.


Proof of Concept


  1. A seller creates Order X with priceInUSDC significantly below market value.

  2. Legitimate Buyer A submits a transaction to call buyOrder(Order X ID) with a standard gas fee. This transaction enters the mempool.

  3. A Front-running Bot observes Buyer A's transaction in the mempool, recognizes the profitable Order X.

  4. The Bot immediately submits its own buyOrder(Order X ID) transaction with a significantly higher gas fee.

  5. A miner processes the Bot's transaction first due to the higher gas fee. The Bot successfully fills Order X at the favorable price.

  6. Buyer A's transaction is then processed. However, Order X is now inactive (order.isActive is false), causing Buyer A's transaction to revert or fail. Buyer A misses the profitable trade.


Recommended Mitigation


Implement slippage control in the buyOrder function. This allows the buyer to specify a maximum acceptable price, ensuring their transaction reverts if the execution price (due to front-running or other market shifts) exceeds their tolerance.

Diff

function buyOrder(
uint256 _orderId,
+ uint256 _maxPriceInUSDCAllowed // New parameter: maximum total USDC buyer is willing to pay
) 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();
+ // Calculate the actual cost based on the order's price
+ uint256 actualCost = order.priceInUSDC; // Assuming priceInUSDC is already total price
+
+ // Check if the order's price exceeds the buyer's maximum acceptable price
+ if (actualCost > _maxPriceInUSDCAllowed) {
+ revert InvalidPrice(); // Or a more specific error like "SlippageExceeded()"
+ }
+
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);
}

Note: If priceInUSDC represents the price per unit and buyOrder implies buying the full amountToSell, then _maxPriceInUSDCAllowed should represent the max_total_price. If buyOrder allows partial fills (which it doesn't currently), then _maxPriceInUSDCAllowed would be a price per unit. The provided solution assumes priceInUSDC is the total price for amountToSell and the buyer implicitly buys the full amount.


Reference Files:

  • src/OrderBook.sol

  • test/TestOrderBook.t.sol

Updates

Lead Judging Commences

yeahchibyke Lead Judge
about 1 month ago
yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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