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

Non-standard ERC20 Tokens Break Accounting and Lock Funds in OrderBook.sol

ishwar

Root + Impact

Description

Normal Behavior:
When a user creates or amends a sell order, the contract calls safeTransferFrom to pull the specified amount of tokens from the user and records this amount as amountToSell in the order struct. When a buyer fills the order, the contract transfers exactly this recorded amount to the buyer.

Specific Issue:
The contract assumes that all ERC20 tokens are standard and that the amount requested in safeTransferFrom will be received by the contract. However, with fee-on-transfer tokens, rebasing tokens, or other non-standard ERC20s, the contract may receive less than the requested amount, while still recording the full amount in the order. This can result in unfillable orders, failed trades, or locked funds.

function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
...
IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
// @> The contract assumes it received _amountToSell tokens, but may have received less.
orders[orderId] = Order({
...
amountToSell: _amountToSell, // @> Records the requested amount, not the actual tokens received.
...
});
...
}

Risk

Likelihood:

  • This will occur whenever a non-standard ERC20 token (such as a fee-on-transfer or rebasing token) is added to the allowed list via setAllowedSellToken.

  • The owner may accidentally or intentionally allow such tokens, or token behavior may change after allowlisting.

Impact:

  • Buyers will be unable to fill orders because the contract does not actually possess the promised amount, causing transaction failures.

  • Funds may become locked in the contract, resulting in stuck orders and user losses.

Proof of Concept

// Step-by-step explanation of how this vulnerability can be exploited:
//
// 1. The contract owner adds a fee-on-transfer token to the allowed list using setAllowedSellToken().
// 2. A seller creates a sell order for 100 tokens (the token deducts a 10% fee on every transfer).
// 3. The contract receives only 90 tokens but records 100 in the order struct.
// 4. When a buyer tries to fill the order, the contract attempts to transfer 100 tokens to the buyer,
// but only 90 are available. This causes the transaction to revert, leaving both buyer and seller stuck.
//
// In this scenario, funds are locked in the contract and the order is unfillable.

Explanation:
This Proof of Concept demonstrates how a malicious or simply non-standard ERC20 token (such as a fee-on-transfer or rebasing token) can cause the contract to mis-account tokens, leading to unfillable and stuck orders.

Recommended Mitigation

- IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
- orders[orderId] = Order({
- ...
- amountToSell: _amountToSell,
- ...
- });
+ uint256 before = IERC20(_tokenToSell).balanceOf(address(this));
+ IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
+ uint256 after = IERC20(_tokenToSell).balanceOf(address(this));
+ uint256 received = after - before;
+ orders[orderId] = Order({
+ ...
+ amountToSell: received,
+ ...
+ });

Explanation:
By measuring the token balance before and after the transfer, the contract records the actual amount received, regardless of token mechanics. This ensures that orders can always be filled and prevents the possibility of locked or unaccounted funds.

Updates

Lead Judging Commences

yeahchibyke Lead Judge
4 months ago
yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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