Submission Details
Impact:
Likelihood:
[L-1] buyOrder Function Vulnerable to Reentrancy - Fee Accounting Manipulation
Root + Impact
Description
-
Describe the normal behavior in one or more sentences
-
Explain the specific issue or problem in one or more sentences
## Summary
The `buyOrder` function is vulnerable to reentrancy attacks due to state
updates occurring after external calls. While the core order filling
logic is protected, the fee accounting can be manipulated by malicious token
contracts, potentially leading to inaccurate fee tracking.
## Vulnerability Details
The `buyOrder` function updates state after external calls, creating a reentrancy vulnerability:
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; // State updated early
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; // State updated after external calls
emit OrderFilled(_orderId, msg.sender, order.seller);
}
--Issues:
- `totalFees` updated after external `safeTransfer` calls
- Malicious token contracts can reenter before fee accounting
- Fee tracking can become inaccurate
- Potential for fee manipulation attacks
## Impact
- Fee accounting manipulation: Malicious tokens can affect fee tracking
- Inaccurate protocol fees: `totalFees` may not reflect actual fees collected
- Potential for fee extraction: Sophisticated attacks could manipulate fee calculations
- Best practice violation: Missing reentrancy protection
## Affected Functions
- `buyOrder()` - Lines 193-210 (reentrancy vulnerability)
Risk
Likelihood:
-
Reason 1 // Describe WHEN this will occur (avoid using "if" statements)
-
Reason 2
Impact:
-
Impact 1
-
Impact 2
Proof of Concept
# Proof of Concept
1. User calls buyOrder with malicious token
2. Malicious token triggers reentrancy during safeTransfer
3. Second buyOrder call executes before totalFees is updated
4. Fee accounting becomes inaccurate
5. Protocol fee tracking is compromised
## The Problem
State is updated after external calls, allowing reentrancy:
// Lines 193-210: buyOrder()
function buyOrder(uint256 _orderId) public {
// ... validation checks ...
order.isActive = false; // Protected against double-filling
// External calls that can trigger reentrancy
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
totalFees += protocolFee; // Vulnerable to reentrancy
}
## Attack Scenario
- Malicious token implements `_beforeTokenTransfer` hook
- Hook calls back to `buyOrder` before `totalFees` update
- Second execution manipulates fee calculations
- Fee accounting becomes inaccurate
- Protocol loses track of actual fees collected
Recommended Mitigation
--Vulnerable Code:
// Lines 193-210: buyOrder()
function buyOrder(uint256 _orderId) public {
// ... validation and state updates ...
// External calls that can trigger reentrancy
iUSDC.safeTransferFrom(msg.sender, address(this), protocolFee);
iUSDC.safeTransferFrom(msg.sender, order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
totalFees += protocolFee; // State updated after external calls
}
## How to Fix
### Solution 1: Use ReentrancyGuard (Recommended)
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract OrderBook is Ownable, ReentrancyGuard {
function buyOrder(uint256 _orderId) public nonReentrant {
// ... existing logic ...
totalFees += protocolFee; // Protected by reentrancy guard
}
}
Updates
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.