Summary

The redeem function is used by liquidity providers to get their underlying tokens (plus fees they earned) back. The redeem function has a check that the token must be an allowed token or the redeem will not work. But the setAllowedToken function (which is used to both add and remove allowed tokens) can just change a token from allowed to not allowed and there is no provision for liquidity providers to get their tokens back (unless the owner switches the token back to allowed, but that might not be an attractive option if you don't want people to continue being able to flash loan that token for whatever reason).

Vulnerability Details

The redeem function will revert if the token is no longer an allowed token:

This portion of setAllowedToken makes a token not allowed and it is silent as to what should happen with the tokens currently put in as liquidity.

Test

Here is a test I wrote to prove that you can't redeem an unapproved token even if you still have deposits....it uses the setAllowedToken and hasDeposits modifiers. hasDeposits means that liquidityProvider has deposited DEPOSIT_AMOUNT of tokenA.

Impact

LP's liquidity will be stuck in the contract at least temporarily. You can fix it by making the token allowed again but even if you do this it is possible that some people won't get the news that they need to redeem, so unless you are okay with having the token permanently allowed, some people's tokens will be locked. You probably had a reason to no longer have the token as allowed so you won't want to keep it allowed long term, most likely.

Tools Used

Manual review

Recommendations

You could send all the tokens back to liquidity providers as part of the function to change a token to not allowed (although this may not be ideal if you just want to turn off a token's allowed state briefly). Alternatively you can make it so that tokens that were approved but were later disapproved can still use the redeem function through the following changes:

Add an array of asset token addresses that will track all addresses ever approved:

In the setAllowedToken function, push every asset token address that is ever approved to the array (you won't remove the address from this array even if its approval is revoked):

Add a function to check if a particular token was ever an approved token:

Add a modifier revertIfNeverAllowedToken as follows and then apply this new modifier to the redeem function in place of revertIfNotAllowedToken: