deposit() increases the AssetToken exchange rate as if a flash loan fee had been earned, so a deposit-redeem cycle prints money out of other LPs' deposits
Root + Impact
deposit() mints AssetTokens at the current exchange rate and then calls assetToken.updateExchangeRate(calculatedFee), where calculatedFee is the fee a flash loan of the same size would have paid. No fee was actually collected, no flash loan happened, yet the exchange rate, the price at which every AssetToken redeems for underlying, ratchets upward. A depositor can therefore deposit and immediately redeem for more underlying than they put in. The difference is paid out of the vault, i.e. out of the other liquidity providers' deposits, and the cycle can be repeated. (Notably, ThunderLoanUpgraded quietly removes these two lines from deposit, which confirms they do not belong there; the live ThunderLoan still has them.)
Description
AssetToken.updateExchangeRate multiplies the rate by (totalSupply + fee) / totalSupply, so even a modest deposit inflates every outstanding AssetToken's redemption value while the vault only gained the deposited amount itself.
Risk
Likelihood: High - callable by anyone with capital to deposit, no flash loan, no counterparty, no timing conditions; repeatable every block.
Impact: High - each cycle transfers value from existing LPs to the cycling depositor (verified ~0.15 tokenA profit on a 100 tokenA cycle against a 100 tokenA pool, and it scales with deposit size relative to the pool). After the attacker's cycles, the first LP's full redemption reverts because their claim at the inflated rate exceeds the underlying actually left in the vault.
Proof of Concept
Verified with Foundry (test passes). Pool holds one LP's 100 tokenA. The attacker deposits 100 tokenA and immediately redeems all their AssetTokens, receiving ~100.15 tokenA back. The original LP then cannot redeem in full: the vault cannot cover their claim at the inflated exchange rate.
Recommended Mitigation
Remove the exchange-rate update from deposit (as the upgraded contract already does). The exchange rate should only increase when an actual flash loan fee has been paid into the pool:
Lead Judging Commences
[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); } ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.