Description

The OrderBook contract implicitly assumes that all ERC20 tokens it interacts with (including core tokens like wETH, wBTC, wSOL, USDC, and any others whitelisted by the owner) behave as standard ERC20s. This means it expects that when a transfer or transferFrom call is made with a specified amount, the exact same amount is always received by the destination address.

This assumption does not hold true for all ERC20 tokens in the wild, such as "fee-on-transfer" tokens (which deduct a percentage or fixed amount during each transfer) or "rebasing tokens" (whose balances dynamically change over time).

​

Risk

Likelihood: Medium

This risk materializes if the contract's owner uses setAllowedSellToken to whitelist a non-standard ERC20 token, and subsequently, users interact with that token within the order book. The risk becomes high if the owner accidentally or maliciously whitelists such a token.

​

Impact: Low to Medium

Interaction with non-standard tokens can lead to incorrect internal accounting, unexpected balance discrepancies, or financial loss for users and the protocol. For example, if a fee-on-transfer token is traded, the amount received by the contract or the seller might be less than the calculated amount, causing a deficit or failed transactions.

​

function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {

if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken(); // Cannot allow null or USDC itself

allowedSellToken[_token] = _isAllowed; // @> Line 185: This line, combined with the lack of checks for non-standard token behaviors, is where the assumption manifests.

​

emit TokenAllowed(_token, _isAllowed);

}

​

​

​

Explanation:

Conceptual PoC (using a hypothetical fee-on-transfer token):

Assume the owner uses setAllowedSellToken to whitelist MaliciousFeeToken which deducts a 5% fee on every transfer.

A seller lists an order for 100 MaliciousFeeToken. When createSellOrder calls MaliciousFeeToken.safeTransferFrom(seller, OrderBook, 100), only 95 MaliciousFeeToken are actually transferred to the OrderBook contract due to the 5% fee.

The OrderBook contract's internal state still records order.amountToSell = 100.

When a buyer fills this order, OrderBook attempts to safeTransfer 100 MaliciousFeeToken to the buyer. This transaction will either revert (if the OrderBook contract doesn't have enough balance) or, in the case of some non-standard tokens, might succeed but only transfer the available 95 tokens, causing the buyer to receive less than expected without a clear revert.

function setAllowedSellToken(address _token, bool _isAllowed) external onlyOwner {

if (_token == address(0) || _token == address(iUSDC)) revert InvalidToken(); // Cannot allow null or USDC itself

allowedSellToken[_token] = _isAllowed;

emit TokenAllowed(_token, _isAllowed);

​

​

​

Explanation:

For a more robust solution that actively defends against non-standard token behaviors, you can implement pre- and post-transfer balance checks. Before and after any token transfer (e.g., in createSellOrder, buyOrder, amendSellOrder), you would read the actual balance of the token in the contract or the recipient's address. You then compare the change in balance to the amount that was supposed to be transferred.

If the actual amount received/sent is less than expected (e.g., due to a fee-on-transfer), the transaction can be reverted, or the discrepancy can be handled explicitly.

This method effectively validates the behavior of the token during the transfer operation itself.

Why it helps: This approach provides a strong programmatic defense. It ensures that even if a non-standard token is whitelisted, the contract's logic verifies that the expected amount of tokens moved, thereby protecting against unexpected deductions or rebase issues. It makes the contract more resilient to varying ERC20 implementations.

Owner Vigilance and Due Diligence:

Regardless of technical mitigations, emphasizing the owner's responsibility to perform due diligence on any new token intended for whitelisting is crucial. This includes researching the token's contract, auditing its code if possible, and testing its behavior in a controlled environment.

By combining clear documentation and, if necessary, implementing runtime checks, you significantly reduce the risk associated with interactions with non-standard ERC20 tokens.

​

​

​

//Additional Strategy: If supporting non-standard tokens becomes a requirement, implement robust pre- and post-transfer balance checks to explicitly verify the amount received matches the amount sent.

For example:

```

Solidity

​

// Within buyOrder or createSellOrder before or after transfers:

// uint256 balanceBefore = IERC20(tokenAddress).balanceOf(address(this));

// IERC20(tokenAddress).safeTransferFrom(sender, receiver, amount);

// uint256 balanceAfter = IERC20(tokenAddress).balanceOf(address(this));

// require(balanceAfter - balanceBefore >= amount, "Token transfer amount mismatch");

```

//Documentation: Clearly document that the contract is designed only for standard ERC20 tokens and advise the owner to exercise extreme caution and verify new tokens before whitelisting.