OrderBook
First Flight #43
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Potential Frontrunning risk in `amendSellOrder`

0xshuayb

Root + Impact

Order being active during amendSellOrder poses risk of frontrunning

Description

When the price of a token increases and a seller wants to amend the order price to match the new price of the token, an attacker sees the transaction in the mempool and front runs is, buying the order at the stale price before the amendSellOrder transaction goes through

  • Describe the normal behavior in one or more sentences

A seller should be able to change the price of tokens for sale without risk of the tokens getting sold while waiting for the amend transaction to go through

  • Explain the specific issue or problem in one or more sentences
    Because the state of the order is active, order.isActive throughout the amendSellOrder transaction, there's is risk of front-running by an attacker who sees the amend transaction in the mempool before it goes through

// Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood: Medium

  • Reason 1: When there's a an increase in price of a token and a seller wants to amend the price to match the current price action

Impact: The seller risk selling their orders at prices lower down they desire

Proof of Concept

function test_frontrunAmendSellOrder() public {
// alice creates sell order for wbtc
vm.startPrank(alice);
wbtc.approve(address(book), 2e8);
uint256 aliceId = book.createSellOrder(address(wbtc), 2e8, 180_000e6, 2 days);
vm.stopPrank();
​
// dan sees alice's amendSellOrder in the mempool and frontruns it, buying the order before the amendSellOrder txn goes through
vm.startPrank(dan);
usdc.approve(address(book), 200_000e6);
​
console2.log("Dan's USDC balance before attack:", usdc.balanceOf(dan));
console2.log("Dan's WBTC balance before attack:", wbtc.balanceOf(dan));
​
// Dan successfully buys at the old price of $180,000
book.buyOrder(aliceId);
vm.stopPrank();
​
// alice amends her wbtc sell order which fails because the order has been filled
vm.prank(alice);
book.amendSellOrder(aliceId, 2e8, 200_000e6, 0.5 days);
string memory aliceOrderDetails = book.getOrderDetailsString(aliceId);
console2.log(aliceOrderDetails);
vm.expectRevert(OrderBook.OrderNotActive.selector);
book.amendSellOrder(aliceId, 2e8, 200_000e6, 2 days);
vm.stopPrank();
}
​

Recommended Mitigation

The sell order should be rendered inactive during amendment and back to active when the amendment is done

​
function amendSellOrder(
uint256 _orderId,
uint256 _newAmountToSell,
uint256 _newPriceInUSDC,
uint256 _newDeadlineDuration
) public {
Order storage order = orders[_orderId];
​
.
.
.
+ order.isActive = false;
​
.
.
.
+ order.isActive = true;
​
emit OrderAmended(_orderId, _newAmountToSell, _newPriceInUSDC, newDeadlineTimestamp);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

Amends or cancellation of sell orders can be front-run

When a seller wants to amend or cancel their sell orders, a malicious entity can front-run their transactions and buy out the orders. This can be especially harmful when real-world prices of listed assets fluctuate and sellers want to adjust the prices listed in their orders.

Support

FAQs

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