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

Removing an allowed token permanently locks existing LP funds

joshuapaul1

Root + Impact

Description

Disabling an asset should prevent new deposits and flash loans while preserving existing LPs’ ability to redeem their AssetTokens for underlying assets.

setAllowedToken(token, false) deletes the mapping from the underlying token to its AssetToken. Since redeem() requires the token to remain allowed, existing LPs can no longer redeem. Re-enabling the token creates a new AssetToken contract, which has no relationship to the old AssetTokens or the underlying funds held by the old AssetToken contract.

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);
s_tokenToAssetToken[token] = assetToken;
return assetToken;
} else {
AssetToken assetToken = s_tokenToAssetToken[token];
​
@> delete s_tokenToAssetToken[token];
​
return assetToken;
}
}
function redeem(IERC20 token, uint256 amountOfAssetToken)
external
revertIfZero(amountOfAssetToken)
@> revertIfNotAllowedToken(token)
{
@> AssetToken assetToken = s_tokenToAssetToken[token];
// ...
}

Risk

Likelihood:

  • The owner can disable any supported token at any time through the exposed setAllowedToken(token, false) function.

  • Token removal is a normal operational action when a protocol stops supporting an asset or responds to an asset-specific issue.

Impact:

  • All LPs holding AssetTokens for the removed asset lose access to redemption.

  • Re-enabling the token does not restore access: it deploys a new AssetToken while the old AssetToken and its underlying balance remain stranded.

Proof of Concept

Add this test inside [`test/unit/ThunderLoanTest.t.sol`]

function testRemovingAndReaddingTokenLocksExistingLpFunds() public setAllowedToken {
uint256 depositAmount = 100e18;
​
vm.startPrank(liquidityProvider);
tokenA.mint(liquidityProvider, depositAmount);
tokenA.approve(address(thunderLoan), depositAmount);
thunderLoan.deposit(tokenA, depositAmount);
vm.stopPrank();
​
AssetToken oldAssetToken = thunderLoan.getAssetFromToken(tokenA);
​
// Owner removes tokenA from the protocol.
vm.prank(thunderLoan.owner());
thunderLoan.setAllowedToken(tokenA, false);
​
// Existing LPs can no longer redeem.
vm.prank(liquidityProvider);
vm.expectRevert(
abi.encodeWithSelector(
ThunderLoan.ThunderLoan__NotAllowedToken.selector,
address(tokenA)
)
);
thunderLoan.redeem(tokenA, type(uint256).max);
​
// Re-enabling tokenA creates an entirely new AssetToken.
vm.prank(thunderLoan.owner());
AssetToken newAssetToken = thunderLoan.setAllowedToken(tokenA, true);
​
assertNotEq(address(oldAssetToken), address(newAssetToken));
​
// Underlying tokens remain in the old AssetToken contract.
assertGt(tokenA.balanceOf(address(oldAssetToken)), 0);
​
// The LP owns no shares in the newly created AssetToken.
assertEq(newAssetToken.balanceOf(liquidityProvider), 0);
}

Recommended Mitigation

Do not delete the underlying-token-to-AssetToken mapping while the AssetToken has outstanding supply or underlying balance.

Maintain the mapping permanently and use a separate status flag to disable new deposits and flash loans while continuing to permit redemptions:

mapping(IERC20 => bool) private s_depositsEnabled;
​
function disableNewDeposits(IERC20 token) external onlyOwner {
s_depositsEnabled[token] = false;
}

Alternatively, require totalSupply() == 0 and the AssetToken’s underlying balance to be zero before allowing a token to be removed.

Updates

Lead Judging Commences

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