Protocol fee can be bypassed for low-value orders.
Description
Each time someone buys an order, the contract calculate fees and subtract them from the order's price. However, for order's where the priceInUSDC <= 33 the contract subtract 0 fees because of integer division truncation issue.
The protocol calculates protocol fees using this formula in OrderBook.sol::buyOrder()
uint256 protocolFee = (order.priceInUSDC * FEE) / PRECISION;
FEE = 3
PRECISION = 100
Since solidity doesn't support floating point or decimal division, any fee calculation that results in a fractional number less than 1 will be truncated to .
Risk
Likelihood: High
Impact: High
Proof of Concept
Replace the test_buyOrder() in TestOrderBook.t.sol with this:
function test_buyOrder() public {
vm.startPrank(alice);
wbtc.approve(address(book), 2e8);
uint256 aliceId = book.createSellOrder(address(wbtc), 2e8, 33, 2 days);
vm.stopPrank();
assert(aliceId == 1);
assert(wbtc.balanceOf(alice) == 0);
assert(wbtc.balanceOf(address(book)) == 2e8);
vm.startPrank(bob);
weth.approve(address(book), 2e18);
uint256 bobId = book.createSellOrder(address(weth), 2e18, 33, 2 days);
vm.stopPrank();
assert(bobId == 2);
assert(weth.balanceOf(bob) == 0);
assert(weth.balanceOf(address(book)) == 2e18);
vm.startPrank(clara);
wsol.approve(address(book), 2e18);
uint256 claraId = book.createSellOrder(address(wsol), 2e18, 33, 2 days);
vm.stopPrank();
vm.startPrank(dan);
usdc.approve(address(book), 200_000e6);
book.buyOrder(aliceId);
book.buyOrder(bobId);
book.buyOrder(claraId);
vm.stopPrank();
assert(book.totalFees() == 0);
vm.prank(owner);
vm.expectRevert();
book.withdrawFees(owner);
assert(usdc.balanceOf(owner) == 0);
}
forge test --match-test buyOrder
Recommended Mitigation
Easiest fix is to implement minimum price > 33 policy
function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
if (!allowedSellToken[_tokenToSell]) revert InvalidToken();
if (_amountToSell == 0) revert InvalidAmount();
- if (_priceInUSDC == 0) revert InvalidPrice();
+ if (_priceInUSDC <= 33) revert InvalidPrice();
if (_deadlineDuration == 0 || _deadlineDuration > MAX_DEADLINE_DURATION) revert InvalidDeadline();
uint256 deadlineTimestamp = block.timestamp + _deadlineDuration;
uint256 orderId = _nextOrderId++;
IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
// Store the order
orders[orderId] = Order({
id: orderId,
seller: msg.sender,
tokenToSell: _tokenToSell,
amountToSell: _amountToSell,
priceInUSDC: _priceInUSDC,
deadlineTimestamp: deadlineTimestamp,
isActive: true
});
emit OrderCreated(orderId, msg.sender, _tokenToSell, _amountToSell, _priceInUSDC, deadlineTimestamp);
return orderId;
}
