OrderBook

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

Total Fee state is updated after external transferFrom interaction making the buyOrder vulnerable to reentracy

Description

When a user buys an active sellOrder, a certain fee is deducted from this user and stored as protocolFee.

The function proceeds to transfer this fee from the buyer to the OrderBook contract. The totalFees state variable which tracks the accumulation of protocolFee is only updated after this transfer. This does not follow the Checks Effects and Interaction best practice

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();
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);

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();
order.isActive = false;
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
uint256 sellerReceives = order.priceInUSDC - protocolFee;
// @written user receives less than amount listed in order
+ totalFees += 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;
// @audit CEI attack vector
emit OrderFilled(_orderId, msg.sender, order.seller);
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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