Summary
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.
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;
}
}
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.
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();
}
Tools Used
--Foundry
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.
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;
}
}
