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

Missing Precondition Checks for `allowance` and `balanceOf` in `OrderBook::buyOrder()` function, leading to confusion and poor UX.

mozi

Missing Precondition Checks for allowance and balanceOf in OrderBook::buyOrder() function

Description

The OrderBook contract is designed as a peer-to-peer trading system where sellers lock ERC20 tokens (such as wETH, wBTC, or wSOL) and list them at a desired USDC price. Buyers are expected to fill these orders by paying USDC.

The protocol documentation claims that:

"Buyers can fill them directly on-chain."

While this is technically true, in practice the buyer must first grant sufficient USDC allowance to the OrderBook contract before calling buyOrder(). The current implementation does not validate whether the buyer has approved the necessary USDC amount or has enough balance.

As a result, any buyer who attempts to fill an order without setting the allowance beforehand will experience a hard revert from the USDC token contract — typically with an message such as "ERC20: insufficient allowance". This leads to failed trades and broken UX, despite the documentation suggesting a straightforward interaction.

//i function did not used internally consider change to external
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;
​
//q no validation for balanceOf buyer?
//q no allowance checks for buyer?
​
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:

  • Buyers will commonly attempt to fill an order without first approving USDC, especially if interacting via Etherscan or a minimal frontend.

  • The documentation implies a one-step on-chain interaction, encouraging users to call buyOrder() directly without realizing an approve() is required.

Impact:

  • Transactions will revert due to insufficient allowance or balance without any clear custom error from the protocol contract, leading to confusion and poor UX.

  • This breaks the core trading flow promised by the protocol, and makes it unusable for most first-time users unless a frontend explicitly manages allowance logic.

Proof of Concept

// User does NOT approve USDC before calling buyOrder
// Assume order.priceInUSDC = 100 USDC
​
// Step 1: No approve() called
// Step 2: User calls this directly (e.g. via Etherscan)
orderBook.buyOrder(orderId);
​
// This will revert with:
"ERC20: insufficient allowance"

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;
​
+ if (iUSDC.allowance(msg.sender, address(this)) < order.priceInUSDC) revert InsufficientAllowance();
+ if (iUSDC.balanceOf(msg.sender) < order.priceInUSDC) revert InsufficientBalance();
​
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);
}

Adding checks for allowance and balanceOf buyer and revert with custom error

Updates

Lead Judging Commences

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.