Thunder Loan

AI First Flight #7
Beginner FriendlyFoundryDeFiOracle
EXP
View results
Submission Details
Severity: medium
Valid

Removing an allowed token (`setAllowedToken(token, false)`) permanently locks LP principal — re-allow forks to an empty vault

Description

setAllowedToken only toggles a mapping entry. The AssetToken contract that holds the vaulted underlying is
created once and never destroyed, migrated, or emptied when a token is delisted. Every access path
(deposit/redeem/flashloan) resolves the asset through the live mapping, so once the entry is deleted the
entire vault becomes unreachable — and re-adding the token creates a brand-new empty AssetToken, permanently
orphaning the old vault's funds. There is no withdrawal function that can reach the orphaned address.

// src/protocol/ThunderLoan.sol:228-245 — @> deleting the mapping entry does NOT empty the AssetToken vault
function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) {
if (allowed) {
if (address(s_tokenToAssetToken[token]) != address(0)) {
revert ThunderLoan__AlreadyAllowed();
}
...
AssetToken assetToken = new AssetToken(address(this), token, name, symbol); // @> fresh EMPTY vault
s_tokenToAssetToken[token] = assetToken;
...
} else {
AssetToken assetToken = s_tokenToAssetToken[token];
delete s_tokenToAssetToken[token]; // @> vault (and its balance) orphaned in place
...
}
}
// src/protocol/ThunderLoan.sol:161-178 — @> redeem resolves the vault ONLY through the current mapping
function redeem(IERC20 token, uint256 amountOfAssetToken) external revertIfZero(amountOfAssetToken) revertIfNotAllowedToken(token) {
AssetToken assetToken = s_tokenToAssetToken[token]; // @> old vault unreachable after delete
...
assetToken.transferUnderlyingTo(msg.sender, amountUnderlying);
}

The AssetToken implementation has no public function and no onlyThunderLoan path that can withdraw from the
orphaned vault, and ThunderLoan has no migration/liquidation flow for delisted tokens (both ThunderLoan.sol and
ThunderLoanUpgraded.sol share this design).

Root Cause

setAllowedToken separates "registration" from "custody". The vaulted principal lives in a per-token contract,
but every access path depends on a mapping entry that setAllowedToken(false) deletes. Delisting a token — a
routine admin action — strands the underlying with no withdrawal, migration, or emergency-exit path.

Risk

Likelihood: Requires the owner to call setAllowedToken(token, false) (documented admin action, e.g.
delisting a token after an incident). Relies on a trusted-actor trigger; downrate if the contest's trust model
treats all admin actions as benign.

Impact: Permanent, unrecoverable loss of access to all LP principal in that token's vault (stuck, not
recoverable by anyone, including the owner). Re-adding the token boots an empty vault, so the market "works
again" while the old balances are frozen forever. In v1/v2 release states both are affected.

Proof of Concept

test/poc/PocSetAllowedTokenFundLock.t.sol (PASS):

function test_PoC_Disallow_LocksLPFundsPermanently() public {
tokenA.mint(lp, 10_000e18);
vm.startPrank(lp);
tokenA.approve(address(tl), type(uint256).max);
tl.deposit(tokenA, 10_000e18); // LP vaults 10k tokenA
vm.stopPrank();
address oldVault = address(asset);
assertEq(tokenA.balanceOf(oldVault), 10_000e18);
vm.prank(address(this));
tl.setAllowedToken(tokenA, false); // @> normal delist
vm.expectRevert(); // @> redeem now impossible: NotAllowedToken
vm.prank(lp);
tl.redeem(tokenA, type(uint256).max);
vm.prank(address(this));
AssetToken newAsset = tl.setAllowedToken(tokenA, true); // re-allow: forks an EMPTY vault
assertTrue(address(newAsset) != oldVault);
assertEq(newAsset.totalSupply(), 0);
assertEq(tokenA.balanceOf(oldVault), 10_000e18); // @> old vault still holds the 10k
assertEq(asset.balanceOf(lp), 10_000e18); // @> LP's shares are phantom in an unreachable pool
assertEq(newAsset.balanceOf(lp), 0); // @> no path to the old funds
}

Run: forge test --match-contract PocSetAllowedTokenFundLock -vv.

Recommended Mitigation

  • Do not orphan vaults: on setAllowedToken(false), either keep the mapping so LPs can still redeem (only
    block new deposits/flash loans), or add an explicit owner/any-user withdrawDelisted(token, asset) that
    allows each LP to burn their shares and pull the underlying.

  • Re-allowing a previously-delisted token should reuse (and re-enable) the existing AssetToken rather than
    deploying a new one.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[M-01] 'ThunderLoan::setAllowedToken' can permanently lock liquidity providers out from redeeming their tokens

## Description If the 'ThunderLoan::setAllowedToken' function is called with the intention of setting an allowed token to false and thus deleting the assetToken to token mapping; nobody would be able to redeem funds of that token in the 'ThunderLoan::redeem' function and thus have them locked away without access. ## Vulnerability Details If the owner sets an allowed token to false, this deletes the mapping of the asset token to that ERC20. If this is done, and a liquidity provider has already deposited ERC20 tokens of that type, then the liquidity provider will not be able to redeem them in the 'ThunderLoan::redeem' function. ```solidity function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) { if (allowed) { if (address(s_tokenToAssetToken[token]) != address(0)) { revert ThunderLoan__AlreadyAllowed(); } string memory name = string.concat("ThunderLoan ", IERC20Metadata(address(token)).name()); string memory symbol = string.concat("tl", IERC20Metadata(address(token)).symbol()); AssetToken assetToken = new AssetToken(address(this), token, name, symbol); s_tokenToAssetToken[token] = assetToken; emit AllowedTokenSet(token, assetToken, allowed); return assetToken; } else { AssetToken assetToken = s_tokenToAssetToken[token]; @> delete s_tokenToAssetToken[token]; emit AllowedTokenSet(token, assetToken, allowed); return assetToken; } } ``` ```solidity function redeem( IERC20 token, uint256 amountOfAssetToken ) external revertIfZero(amountOfAssetToken) @> revertIfNotAllowedToken(token) { AssetToken assetToken = s_tokenToAssetToken[token]; uint256 exchangeRate = assetToken.getExchangeRate(); if (amountOfAssetToken == type(uint256).max) { amountOfAssetToken = assetToken.balanceOf(msg.sender); } uint256 amountUnderlying = (amountOfAssetToken * exchangeRate) / assetToken.EXCHANGE_RATE_PRECISION(); emit Redeemed(msg.sender, token, amountOfAssetToken, amountUnderlying); assetToken.burn(msg.sender, amountOfAssetToken); assetToken.transferUnderlyingTo(msg.sender, amountUnderlying); } ``` ## Impact The below test passes with a ThunderLoan\_\_NotAllowedToken error. Proving that a liquidity provider cannot redeem their deposited tokens if the setAllowedToken is set to false, Locking them out of their tokens. ```solidity function testCannotRedeemNonAllowedTokenAfterDepositingToken() public { vm.prank(thunderLoan.owner()); AssetToken assetToken = thunderLoan.setAllowedToken(tokenA, true); tokenA.mint(liquidityProvider, AMOUNT); vm.startPrank(liquidityProvider); tokenA.approve(address(thunderLoan), AMOUNT); thunderLoan.deposit(tokenA, AMOUNT); vm.stopPrank(); vm.prank(thunderLoan.owner()); thunderLoan.setAllowedToken(tokenA, false); vm.expectRevert(abi.encodeWithSelector(ThunderLoan.ThunderLoan__NotAllowedToken.selector, address(tokenA))); vm.startPrank(liquidityProvider); thunderLoan.redeem(tokenA, AMOUNT_LESS); vm.stopPrank(); } ``` ## Recommendations It would be suggested to add a check if that assetToken holds any balance of the ERC20, if so, then you cannot remove the mapping. ```diff function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) { if (allowed) { if (address(s_tokenToAssetToken[token]) != address(0)) { revert ThunderLoan__AlreadyAllowed(); } string memory name = string.concat("ThunderLoan ", IERC20Metadata(address(token)).name()); string memory symbol = string.concat("tl", IERC20Metadata(address(token)).symbol()); AssetToken assetToken = new AssetToken(address(this), token, name, symbol); s_tokenToAssetToken[token] = assetToken; emit AllowedTokenSet(token, assetToken, allowed); return assetToken; } else { AssetToken assetToken = s_tokenToAssetToken[token]; + uint256 hasTokenBalance = IERC20(token).balanceOf(address(assetToken)); + if (hasTokenBalance == 0) { delete s_tokenToAssetToken[token]; emit AllowedTokenSet(token, assetToken, allowed); + } return assetToken; } } ```

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!