OrderBook
First Flight #43
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Critical Fee Logic Bug in buyOrder() Causes Buyers to Overpay

mohankrishkotte

Critical Fee Logic Bug in buyOrder() Causes Buyers to Overpay

Description

  • When a buyer executes buyOrder(), they should pay the full order.priceInUSDC, and the protocol deducts a 3% fee from the seller’s proceeds and the seller should receive 97% of the order price, and the protocol should retain 3% as fees.

  • The current implementation forces the buyer to pay an extra 3% fee on top of the order price, while the seller still receives the full amount, resulting in buyers overpaying by 3%, while the protocol loses its intended fee revenue.

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

Risk

Likelihood:

  • Every buyOrder() transaction is affected

  • Buyers will notice the discrepancy when paying extra USDC.

Impact:

  • Buyers lose 3% more funds than intended.

  • Protocol fails to collect fees, losing revenue in intended way

Proof of Concept

This test:

  1. Creates a sell order (1 WBTC for 50,000 USDC)

  2. Records initial balances

  3. Executes the buyOrder function

  4. Calculates and logs the actual transfers

  5. Shows the buyer is paying 51,500 USDC (50,000 + 1,500 fee) instead of just 50,000 USDC

The key proof is in the final assertion where we see:

  • Buyer paid 103% of order price (51,500 instead of 50,000)

  • While seller and protocol amounts are mathematically correct, the economic burden is wrong

  • This confirms the fee is being added to buyer's payment rather than deducted from seller's proceeds

function test_BuyerOverpaysDueToFeeBug() public {
// Setup - Alice creates a WBTC sell order
vm.prank(alice);
wbtc.approve(address(book), 1e8); // 1 WBTC
vm.prank(alice);
uint256 orderId = book.createSellOrder(address(wbtc), 1e8, 50_000e6, mdd); // 1 WBTC for 50,000 USDC
​
// Pre-transaction balances
uint256 buyerInitialUSDC = usdc.balanceOf(dan);
uint256 sellerInitialUSDC = usdc.balanceOf(alice);
uint256 protocolInitialUSDC = usdc.balanceOf(address(book));
​
// Dan buys the order
vm.prank(dan);
usdc.approve(address(book), type(uint256).max);
vm.prank(dan);
book.buyOrder(orderId);
​
// Post-transaction balances
uint256 buyerPaid = buyerInitialUSDC - usdc.balanceOf(dan);
uint256 sellerReceived = usdc.balanceOf(alice) - sellerInitialUSDC;
uint256 protocolReceived = usdc.balanceOf(address(book)) - protocolInitialUSDC;
​
console2.log("Buyer paid:", buyerPaid);
console2.log("Seller received:", sellerReceived);
console2.log("Protocol received:", protocolReceived);
​
// Expected values if working correctly:
// Buyer should pay: 50,000 USDC
// Seller should receive: 48,500 USDC (50,000 - 3%)
// Protocol should receive: 1,500 USDC (3% of 50,000)
​
// Actual buggy behavior:
assertEq(buyerPaid, 51_500e6, "Buyer overpaid by 3%"); // 50,000 + 1,500 fee
assertEq(sellerReceived, 48_500e6, "Seller received correct amount");
assertEq(protocolReceived, 1_500e6, "Protocol received correct fee");
​
// The bug is proven because:
// 1. Buyer paid 51,500 USDC (50,000 + 1,500 fee)
// 2. This is 3% more than the order price of 50,000 USDC
// 3. While protocol gets its fee, it's unfairly taken from buyer instead of seller
}

Recommended Mitigation

Bug:
The original code made buyers pay twice:

  1. 3% fee to protocol

  2. 97% to seller
    → Buyer overpaid by 3% (paid 103% total)

Fix:
Now buyers pay once (100% to contract), then:

  1. Contract sends 97% to seller

  2. Keeps 3% as fee
    → Buyer pays exact price (100%), seller effectively pays the fee

function buyOrder(uint256 _orderId) public {
// ...
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);
+ iUSDC.safeTransferFrom(msg.sender, address(this), order.priceInUSDC);
+ iUSDC.safeTransfer(order.seller, sellerReceives);
​
totalFees += protocolFee;
// ...
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
9 days ago
yeahchibyke Lead Judge 8 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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