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

setAllowedToken(false) permanently locks existing LPs' principal (no migration/withdrawal path)

clumsybeary

Description

Normal behavior: delisting a token should stop NEW deposits/loans while existing LPs keep the ability to withdraw their principal.

The issue: setAllowedToken(token, false) only deletes the s_tokenToAssetToken mapping (ThunderLoan.sol:240). Every exit path — redeem() and flashloan() — is guarded by revertIfNotAllowedToken, which reverts once the mapping is gone (ThunderLoan.sol:161-178 / 121-126). The existing AssetToken keeps holding all deposited underlying, but LPs have no self-service way to get it back. Re-listing the token deploys a brand-new EMPTY AssetToken (exchange rate reset to 1e18), orphaning the old shares entirely.

function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) {
...
} else {
AssetToken assetToken = s_tokenToAssetToken[token];
@> delete s_tokenToAssetToken[token]; // mapping gone; no migration, no sweep, no timelock
emit AllowedTokenSet(token, assetToken, allowed);
return assetToken;
}
}
​
function redeem(...) external revertIfNotAllowedToken(token) { ... } // hard-reverts post-delist

Lifecycle check (required for delist-type admin actions): where does existing user state go? Nowhere. Is there a compensation/migration path? None.

Risk

Likelihood: Medium.

  • Reason 1: Triggers when the owner delists a token — the allowed=false branch is a deliberate, built-in protocol feature (the function ships with it), and token support rotation is normal operations; no admin mistake is required beyond using the feature as designed.

  • Reason 2: Requires the token to actually hold LP funds at delist time; for unused tokens the impact is zero.

Impact:

  • Impact 1: 100% of the delisted token's LP principal is permanently locked in the orphaned AssetToken contract (no owner sweep function exists either).

  • Impact 2: Re-listing does not restore access — old shares are stranded on the deleted AssetToken address while the new one starts empty, permanently splitting accounting.

Proof of Concept

Foundry test: test/poc/PocDelistBrick.t.sol (PoC file added under test/poc/ in the contest repo). LP deposits 1,000e18 tokenA; owner delists the token:

LP deposited (tokenA): 1000000000000000000000
LP balance after delist+relist: 0
LP shares on new AssetToken: 0

Actual vs expected: redeem() reverts after delist (ThunderLoan__NotAllowedToken) — expected withdrawal of 1,000e18, actual 0; after re-listing the LP holds zero shares on the new AssetToken.

Recommended Mitigation

Allow exits for delisted tokens (block only new entry), or migrate funds before deleting the mapping:

function redeem(IERC20 token, uint256 amountOfAssetToken) external revertIfZero(amountOfAssetToken)
- revertIfNotAllowedToken(token)
{
+ // exits must remain possible for delisted tokens
+ AssetToken assetToken = s_tokenToAssetToken[token];
+ if (address(assetToken) == address(0)) { /* resolve legacy AssetToken from a delistedToken registry */ }
...
}

Minimal alternative: keep a s_delistedTokenToAssetToken registry written on delist, and have redeem() fall back to it.Root + Impact

Description

  • Describe the normal behavior in one or more sentences

  • Explain the specific issue or problem in one or more sentences

// Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

  • Impact 1

  • Impact 2

Proof of Concept

​

Recommended Mitigation

- remove this code
+ add this code
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!