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

Fee-on-Transfer Tokens Break OrderBook Accounting

martinyordanov2

Description

  • The OrderBook contract assumes that when calling safeTransfer(amount) or safeTransferFrom(sender, receiver, amount), the receiver will receive exactly amount tokens, which is true for standard ERC20 tokens.

  • Fee-on-transfer tokens deduct a fee during transfers, causing the receiver to get fewer tokens than expected. This breaks the protocol's accounting assumptions and creates multiple attack vectors where buyers pay full price but receive fewer tokens than promised.

function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
// ... validation ...
// Protocol assumes _amountToSell tokens will be deposited
@> IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
orders[orderId] = Order({
id: orderId,
seller: msg.sender,
tokenToSell: _tokenToSell,
@> amountToSell: _amountToSell, // This amount may not match actual balance
priceInUSDC: _priceInUSDC,
deadlineTimestamp: deadlineTimestamp,
isActive: true
});
}
​
```solidity
​
function buyOrder(uint256 _orderId) public {
// ... validation ...
// Protocol attempts to transfer recorded amount, not actual balance
@> IERC20(order.tokenToSell).safeTransfer(msg.sender, order.amountToSell);
}

Risk

MediumSeverity

Likelihood: Medium

Impact: Medium

Likelihood:

  • Reason 1 Fee-on-transfer tokens are commonly used in DeFi and users will naturally attempt to list them for sale

  • Reason 2 The protocol allows any token to be added via setAllowedSellToken(), making integration with fee-on-transfer tokens inevitable

  • Reason 3 No validation exists to detect or prevent fee-on-transfer tokens from being listed

Impact:

  • Impact 1 Orders become unfillable when the contract has insufficient tokens due to fees taken during deposit

  • Impact 2 Protocol accounting becomes permanently broken with discrepancies between recorded and actual token balances

  • Impact 3 Sellers can unknowingly create orders that cannot be fulfilled, locking their tokens in the contract

Proof of Concept

  • Step 0: Add the logic of the TransferFeeToken and import it in the test suite.

  • Step 1: Deploy TransferFeeToken
    Create a fee-on-transfer token that burns 10 tokens on every transfer, simulating real-world fee-on-transfer tokens.

  • Step 2: Setup OrderBook
    Allow the TransferFeeToken to be traded on the OrderBook protocol and setup user balances and approvals.

  • Step 3: Create Sell Order
    Ana creates a sell order for 1000 tokens at 100 USDC. Due to the transfer fee, only 990 tokens reach the contract, but the order records 1000 tokens.

  • Step 4: Attempt Purchase
    Dan attempts to buy the order. The protocol tries to transfer 1000 tokens but only has 990 tokens in the contract, causing the transaction to revert with "insufficient-balance".

//TransferFeeToken.sol
​
// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity 0.8.26;
​
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract TransferFeeToken is ERC20 {
uint256 immutable fee;
uint8 tokenDecimals;
​
constructor(uint256 _totalSupply, uint256 _fee, uint8 _tokenDecimals) ERC20("TransferFeeToken", "TFT") {
fee = _fee;
tokenDecimals = _tokenDecimals;
_mint(msg.sender, _totalSupply);
}
​
function transfer(address dst, uint256 amount) public override returns (bool) {
return transferFrom(msg.sender, dst, amount);
}
​
function transferFrom(address src, address dst, uint256 amount) public override returns (bool) {
require(balanceOf(src) >= amount, "insufficient-balance");
// Handle allowance using OpenZeppelin's internal functions
if (src != msg.sender) {
uint256 currentAllowance = allowance(src, msg.sender);
if (currentAllowance != type(uint256).max) {
require(currentAllowance >= amount, "insufficient-allowance");
_approve(src, msg.sender, currentAllowance - amount);
}
}
// Prevent underflow - ensure fee doesn't exceed transfer amount
require(amount >= fee, "transfer amount less than fee");
// Apply fee logic using OpenZeppelin's internal functions
uint256 netAmount = amount - fee;
_transfer(src, dst, netAmount);
// Burn fee tokens (reduce total supply)
if (fee > 0) {
_burn(src, fee);
}
return true;
}
​
function mint(address to, uint256 value) public {
uint256 updateDecimals = uint256(tokenDecimals);
_mint(to, (value * 10 ** updateDecimals));
}
}
​
====================================================
​
// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.0;
​
import {Test, console2} from "forge-std/Test.sol";
import {OrderBook} from "../src/OrderBook.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {MockUSDC} from "./mocks/MockUSDC.sol";
import {MockWBTC} from "./mocks/MockWBTC.sol";
import {MockWETH} from "./mocks/MockWETH.sol";
import {MockWSOL} from "./mocks/MockWSOL.sol";
import {TransferFeeToken} from "./mocks/TransferFeeToken.sol";
​
contract TestOrderBook is Test {
OrderBook book;
​
MockUSDC usdc;
MockWBTC wbtc;
MockWETH weth;
MockWSOL wsol;
TransferFeeToken transferFeeToken;
​
uint256 constant TRANSFER_FEE = 10 * 10 ** 18; // 10 tokens fee
uint256 constant SELL_AMOUNT = 1000 * 10 ** 18; // 1000 tokens
uint256 constant PRICE_IN_USDC = 100 * 10 ** 6; // 100 USDC
​
address owner;
address alice;
address bob;
address clara;
address ana;
address dan;
​
uint256 mdd;
​
function setUp() public {
owner = makeAddr("protocol_owner");
alice = makeAddr("will_sell_wbtc_order");
bob = makeAddr("will_sell_weth_order");
clara = makeAddr("will_sell_wsol_order");
dan = makeAddr("will_buy_orders");
ana = makeAddr("will_sell_transfer_fee_token_order");
​
usdc = new MockUSDC(6);
wbtc = new MockWBTC(8);
weth = new MockWETH(18);
wsol = new MockWSOL(18);
transferFeeToken = new TransferFeeToken(1000000 * 10 ** 18, TRANSFER_FEE, 18);
​
vm.prank(owner);
book = new OrderBook(address(weth), address(wbtc), address(wsol), address(usdc), owner);
​
usdc.mint(dan, 200_000);
wbtc.mint(alice, 2);
weth.mint(bob, 2);
wsol.mint(clara, 2);
transferFeeToken.mint(ana, SELL_AMOUNT);
​
mdd = book.MAX_DEADLINE_DURATION();
}
​
// forge test --mt testFeeToken -vvvv
function testFeeToken() public {
// transferFeeToken must be allowed to be sold
vm.prank(owner);
book.setAllowedSellToken(address(transferFeeToken), true);
​
// Setup approvals
vm.prank(ana);
transferFeeToken.approve(address(book), SELL_AMOUNT);
vm.prank(dan);
usdc.approve(address(book), 200_000e6);
​
// Create a sell order with transferFeeToken
vm.prank(ana);
uint256 anaId = book.createSellOrder(address(transferFeeToken), SELL_AMOUNT, PRICE_IN_USDC, 1 hours);
// Dan buys the order
vm.prank(dan);
vm.expectRevert();
book.buyOrder(anaId);
}
}
​

Recommended Mitigation

function createSellOrder(
address _tokenToSell,
uint256 _amountToSell,
uint256 _priceInUSDC,
uint256 _deadlineDuration
) public returns (uint256) {
// ... existing validation ...
uint256 orderId = _nextOrderId++;
+ // Check balance before and after transfer to handle fee-on-transfer tokens
+ uint256 balanceBefore = IERC20(_tokenToSell).balanceOf(address(this));
IERC20(_tokenToSell).safeTransferFrom(msg.sender, address(this), _amountToSell);
+ uint256 balanceAfter = IERC20(_tokenToSell).balanceOf(address(this));
+ uint256 actualAmountReceived = balanceAfter - balanceBefore;
​
// Store the order with actual received amount
orders[orderId] = Order({
id: orderId,
seller: msg.sender,
tokenToSell: _tokenToSell,
- amountToSell: _amountToSell,
+ amountToSell: actualAmountReceived,
priceInUSDC: _priceInUSDC,
deadlineTimestamp: deadlineTimestamp,
isActive: true
});
​
- emit OrderCreated(orderId, msg.sender, _tokenToSell, _amountToSell, _priceInUSDC, deadlineTimestamp);
+ emit OrderCreated(orderId, msg.sender, _tokenToSell, actualAmountReceived, _priceInUSDC, deadlineTimestamp);
return orderId;
}
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

Appeal created

martinyordanov2 Submitter
about 1 month ago
yeahchibyke Lead Judge
about 1 month ago
martinyordanov2 Submitter
about 1 month ago
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.