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

ThunderLoan::setAllowedToken(false) deletes the AssetToken mapping, so redeem reverts and liquidity providers funds are locked

second_opinion

Root + Impact

Description

  • Liquidity providers deposit tokens and receive AssetTokens, which they redeem later. `redeem` carries the `revertIfNotAllowedToken` modifier, which checks that `s_tokenToAssetToken[token]` is set.

  • When the owner disables a token with `setAllowedToken(token, false)`, the function deletes `s_tokenToAssetToken[token]`, so the token is not allowed anymore and the link to its AssetToken is gone. From then on every `redeem` reverts, and the underlying tokens stay in the AssetToken contract without a way to withdraw them. Allowing the token again deploys a new AssetToken, so the old deposits are not recovered.

function redeem(IERC20 token, uint256 amountOfAssetToken)
external
revertIfZero(amountOfAssetToken)
@> revertIfNotAllowedToken(token) // reverts as soon as the token is no longer allowed
{ ... }
​
function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) {
...
} else {
AssetToken assetToken = s_tokenToAssetToken[token];
@> delete s_tokenToAssetToken[token]; // the only link to the AssetToken is erased
emit AllowedTokenSet(token, assetToken, allowed);
return assetToken;
}
}// Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood:

  • It happens whenever the owner removes a token, which is a normal admin action, with one transaction.

  • The owner does not need to be malicious, a delisting for any reason is enough, and the owner could also freeze deposits on purpose.

Impact:

  • Liquidity providers cannot withdraw their deposits of the disabled token, a permanent loss because re-enabling the token creates a new AssetToken.

  • The owner has a power to lock the funds of all providers of any token, which is a significant centralization and trust risk.

Proof of Concept

The test lets the owner allow tokenA and a liquidity provider deposit 100 tokens. The owner then disables tokenA, which is a normal admin action. When the provider tries to redeem even 1 asset token, the call reverts with ThunderLoan__NotAllowedToken, so the deposit cannot be withdrawn. The AssetToken address was deleted from the mapping, so re-enabling the token creates a new AssetToken and the old balances stay stuck.

function test_R5_disablingTokenLocksLiquidityProviders() public {
_allow();
_deposit(lp, 100e18); // LP deposits 100 tokens
​
vm.prank(thunderLoan.owner());
thunderLoan.setAllowedToken(tokenA, false); // owner disables the token
​
vm.startPrank(lp);
vm.expectRevert(abi.encodeWithSelector(ThunderLoan.ThunderLoan__NotAllowedToken.selector, address(tokenA)));
thunderLoan.redeem(tokenA, 1e18); // LP cannot withdraw anything
vm.stopPrank();
}

Recommended Mitigation

Redeeming must stay possible after a token is disabled. Remove `revertIfNotAllowedToken` from `redeem`, and keep the mapping from the token to its AssetToken when disabling (track the allowed state in a separate flag that only blocks deposits and flash loans).

function redeem(IERC20 token, uint256 amountOfAssetToken)
external
revertIfZero(amountOfAssetToken)
- revertIfNotAllowedToken(token)
{ ... }
​
function setAllowedToken(IERC20 token, bool allowed) external onlyOwner returns (AssetToken) {
...
} else {
AssetToken assetToken = s_tokenToAssetToken[token];
- delete s_tokenToAssetToken[token];
+ s_isAllowed[token] = false; // keep the AssetToken address, only block new deposits and flash loans
...
}
}- remove this code
+ add this code
Updates

Lead Judging Commences

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