sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

chainNue - In case Unitas removing token/pair then user's minted EMC token can't redeem back to USD1 then USDT #149

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

chainNue

medium

In case Unitas removing token/pair then user's minted EMC token can't redeem back to USD1 then USDT

Summary

In case Unitas removing token/pair then user's minted EMC token can't redeem back to USD1 then USDT

Vulnerability Detail

TokenManager has a removeTokensAndPairs() function to remove some tokens and pairs from Unitas protocol.

In some part from the MD file, mention "There is no redemption window, the. protocol is active 24x7x365", in the case of this removing token/pair, it will failed to do so. If the removed token/pairs have totalSupply > 0, these token holder will be impacted, and lose their asset.

To make sure everyone is not left behind, there need to be some kind mechanism, like for example make sure majority of minted token already redeemed, and then pause any token transfer except to redeem.

Impact

EMC token holder will not be able to redeem

Code Snippet

File: TokenManager.sol
132:     function removeTokensAndPairs(
133:         address[] calldata tokens,
134:         address[] calldata pairTokensX,
135:         address[] calldata pairTokensY
136:     ) external onlyTimelock {
137:         _require(pairTokensX.length == pairTokensY.length, Errors.ARRAY_LENGTH_MISMATCHED);
138:
139:         uint256 tokenCount = tokens.length;
140:         uint256 pairCount = pairTokensX.length;
141:
142:         for (uint256 i; i < pairCount; i++) {
143:             _removePair(pairTokensX[i], pairTokensY[i]);
144:         }
145:
146:         for (uint256 i; i < tokenCount; i++) {
147:             _removeToken(tokens[i]);
148:         }
149:     }

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/TokenManager.sol#L132-L149

Tool used

Manual Review

Recommendation

Need to change mechanism, rather than remove a token, it's better to pause and limit the interaction (transfer), or if necessary make the token upgradable.

Duplicate of #89