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

Unrestricted ERC-20 Token Whitelisting

jufel

Description: The owner can call setAllowedSellToken(...) to enable any ERC-20 for trading. A malicious token could implement reentrant hooks in transferFrom/transfer or charge fees on transfer, trapping balances in the contract (e.g. a 1 USDC fee on a 1 000 USDC transfer leaves 1 USDC stuck).

Impact:
– A malicious token could reenter the OrderBook during a transfer hook and manipulate state or deny service to honest users.
– Fee-on-transfer tokens can siphon off protocol revenue by holding back tiny amounts on every trade, leading to unexpected balance leaks.

Proof of Concept: Add the following contract of maliciuos ERC-20 'fee-on-transfer' token:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
/// @notice Fee-on-transfer token that deducts a fee sent to the target contract
contract FeeToken is ERC20 {
uint256 feeBps;
address feeReceiver;
​
constructor(uint256 _feeBps, address _feeReceiver) ERC20("FeeToken", "FEE") {
feeBps = _feeBps;
feeReceiver = _feeReceiver;
}
​
function mint(address to, uint256 amount) public {
_mint(to, amount);
}
​
function transfer(address to, uint256 amount) public override returns (bool) {
uint256 fee = (amount * feeBps) / 10000; // feeBps is in basis points (1 BPS = 0.01%)
uint256 sendAmount = amount - fee;
​
super._transfer(_msgSender(), to, sendAmount);
super._transfer(_msgSender(), feeReceiver, fee);
​
return true;
}
​
function transferFrom(address from, address to, uint256 amount) public override returns (bool) {
uint256 fee = (amount * feeBps) / 10000;
uint256 sendAmount = amount - fee;
​
super._transfer(from, to, sendAmount);
super._transfer(from, feeReceiver, fee);
​
uint256 currentAllowance = allowance(from, msg.sender);
_approve(from, msg.sender, currentAllowance - amount);
​
return true;
}
}

Include the following test in the TestOrderBook.t.sol file:

import {FeeToken} from "./mocks/FeeToken.sol";
​
function testFeeTransferManipulation() public {
FeeToken feeToken = new FeeToken(1000 /* 10% fee */, alice);
feeToken.mint(alice, 100 ether);
​
vm.prank(owner);
book.setAllowedSellToken(address(feeToken), true);
​
vm.startPrank(alice);
feeToken.approve(address(book), 100 ether);
book.createSellOrder(address(feeToken), 100 ether, 100e6, 2 days);
vm.stopPrank();
​
assertEq(feeToken.balanceOf(alice), 10 ether); // 10% fee goes back to alice
assertEq(feeToken.balanceOf(address(book)), 90 ether); // 90% of the tokens are in the book
}

Mitigation:
– Restrict setAllowedSellToken to a pre-approved, audited whitelist (managed off-chain or via a registry).
– Require tokens to adhere strictly to EIP-20 (e.g. via on-chain introspection).
– Add a nonReentrant guard around all state-modifying functions.

Updates

Lead Judging Commences

yeahchibyke Lead Judge
4 months ago
yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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