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

Vulnerability in fee transfer logic in buyOrder()

The Best Audit Group

Root + Impact

Description

  • At the moment the buyer makes two separate calls to pull the price in USDC by transfering protocol fees to the buyer and remaining amount i.e. price_in_usdc - protocol fee to the seller. It shall only make one call rather than two transfer calls which costs gas fees.

// Root cause in the codebase with @> marks to highlight the relevant section
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:

  • This will occur when the user goes in the buyorder function and buys the order

Impact:

  • This will impact the user as he has to provide full USDC price from his wallet and incase if he does not have the amount then transaction will fail


Proof of Concept

Here in the sample step by step scenario you can notice that the buyer should have equivalent USDC balance to process two transfer froms and in case the buyer has low balance then the transaction will revert.

//Sample Scenario
order.priceInUSDC = 1000
​
FEE = 3
​
PRECISION = 100
​
Token being sold: 10 wETH
​
msg.sender: the buyer
​
order.seller: the seller
​
protocolFee = (1000 * 3) / 100 = 30
sellerReceives = 1000 - 30 = 970
​
Buyer send 30 USDC to the protocol
Buyer then sends 970 USDC to the
​
Protocol then sends 10 wETH to the buyer
​
Total fees increased to 30 USDC
​
​
​
​
​

Recommended Mitigation

The way to mitigate is to first pull order price in USDC from the buyer and then send to protocol. The contract will then internally calculate protocol fee and the amount that seller will receive which is then transferred to the seller fro

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;
​
+ iUSDC.safeTransferFrom(msg.sender, address(this), order.priceInUSDC);
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.safeTransfer(order.seller, sellerReceives);
IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell)
totalFees += protocolFee;
emit OrderFilled(_orderId, msg.sender, order.seller);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
about 1 month ago
yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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