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

The depositor may find their underlying tokens locked if they deposit and the token is subsequently removed from the allowedToken list.

eliab256

Root + Impact

Description

  • The protocol allows the owner to add and remove tokens from the allowed list via setAllowedToken.

  • When the owner removes a token from the allowed list by calling setAllowedToken(token, false), the mapping s_tokenToAssetToken is deleted. Since both deposit and redeem use the revertIfNotAllowedToken modifier — which checks isAllowedToken (i.e., whether s_tokenToAssetToken[token] != address(0)) — any depositor who still holds AssetToken for the removed token is permanently unable to call redeem. Their funds remain locked in the AssetToken contract with no recovery mechanism.

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) {
// @audit-issue possible problem with balanceOf external call and reentrancy
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);
}

Risk

Likelihood:

  • The owner calls setAllowedToken(token, false) while depositors still hold AssetToken for that token. There is no check in setAllowedToken that verifies the AssetToken supply is zero before deletion.

Impact:

  • Depositors temporarily lose access to their underlying tokens. The redeem function reverts with ThunderLoan__NotAllowedToken, making it impossible to withdraw funds.

  • Deposit lock can be permanently if owner doesn't call setAllowedToken(token, true)

  • Is not high beacuse owner can always comeback to true.

Proof of Concept

The test simulates a depositor who deposits AssetA to receive LP tokens. Subsequently, the owner disallows TokenA, and we observe that when attempting to call redeem, the function reverts, preventing the depositor from recovering their funds.

contract POCtest is Test {
ThunderLoan thunderLoanImplementation;
MockPoolFactory mockPoolFactory;
ERC1967Proxy proxy;
ThunderLoan thunderLoan;
​
ERC20Mock weth;
ERC20Mock tokenA;
ERC20Mock tokenB;
ERC20Mock6Decimals tokenWith6Decimals;
AssetToken assetToken6Decimals;
AssetToken assetTokenA;
AssetToken assetTokenB;
​
address depositer = makeAddr("depositer");
address flahLoanReceiver = makeAddr("flashLoanReceiver");
address flashLoanAttacker = makeAddr("flashLoanAttacker");
​
function setUp() public virtual {
thunderLoan = new ThunderLoan();
mockPoolFactory = new MockPoolFactory();
​
weth = new ERC20Mock();
tokenA = new ERC20Mock();
tokenB = new ERC20Mock();
tokenWith6Decimals = new ERC20Mock6Decimals();
​
mockPoolFactory.createPool(address(tokenA));
mockPoolFactory.createPool(address(tokenWith6Decimals));
proxy = new ERC1967Proxy(address(thunderLoan), "");
thunderLoan = ThunderLoan(address(proxy));
thunderLoan.initialize(address(mockPoolFactory));
​
assetTokenB = thunderLoan.setAllowedToken(
IERC20(address(tokenB)),
true
);
​
assetToken6Decimals = thunderLoan.setAllowedToken(
IERC20(address(tokenWith6Decimals)),
true
);
​
tokenA.mint(depositer, 50000 * 10 ** tokenA.decimals()); //fund depositer with tokenA
​
//fund depositer with token with 6 decimals
tokenWith6Decimals.mint(
depositer,
5000 * 10 ** tokenWith6Decimals.decimals()
); // 5000 tokens in 6-decimal representation
}
​
function testDeleteAllowedTokenCanLostFundsToDepositer() public {
// This test use Basetest.t.sol setup
assetTokenA = thunderLoan.setAllowedToken(
IERC20(address(tokenA)),
true
);
​
// Deposit some tokenA to get assetTokenA
uint256 depositAmount = 1000 * 10 ** tokenA.decimals();
vm.startPrank(depositer);
tokenA.approve(address(thunderLoan), depositAmount);
thunderLoan.deposit(tokenA, depositAmount);
vm.stopPrank();
​
// assert that the depositer received the correct amount of assetTokenA
uint256 assetTokenDepositerBalance = assetTokenA.balanceOf(depositer);
assertEq(assetTokenDepositerBalance, depositAmount);
​
// Now delete tokenA from allowed tokens
thunderLoan.setAllowedToken(IERC20(address(tokenA)), false);
​
// The depositer can't redeem their assetTokenA for tokenA, because tokenA is no longer allowed
vm.startPrank(depositer);
assetTokenA.approve(address(thunderLoan), assetTokenDepositerBalance);
vm.expectRevert(
abi.encodeWithSelector(
ThunderLoan.ThunderLoan__NotAllowedToken.selector,
address(tokenA)
)
);
thunderLoan.redeem(tokenA, assetTokenDepositerBalance);
vm.stopPrank();
}
}

Recommended Mitigation

The simplest option is elinate reverIfNotAllowedToken modifier on redeem function to keep allowing depotiors have access to their funds but avoiding new deposits.

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) {
// @audit-issue possible problem with balanceOf external call and reentrancy
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);
}
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!