Rebalancing : `removePortfolioToken` can not don ebe fone

[hats-bug-reporter[bot]]

Github username: @aktech297 Twitter username: kaka Submission hash (on-chain): 0x67f5dc39998a033a034d19b0d520a4656bd4d684689d318bc9c388f19f4db647 Severity: low

Description: Description AssetManager can call the removePortfolioToken function and remove the token from the active array.

At the end of the function, _tokenRemoval is called.

Rebalancing.sol#L291-L293

  function _tokenRemoval(address _token, uint256 _tokenBalance) internal {
    if (_tokenBalance == 0) revert ErrorLibrary.BalanceOfVaultIsZero();

The above function check for zero balance, if so, revert the transaction.

But, there are possiblitis that the portfolio token could have zero balance in vault. This could be due to complete withdrawal and other actions.

In this case, even if the assetManager wants to remove the token, it can not happen due to the above balance check.

this is issue only for the funcion call removePortfolioToken. For other function call removeNonPortfolioToken and removePortfolioTokenPartially this will not be an issue.

Impact

Asset manager can not remove the portfolio token.

they are forced to deposit small amount and then call remove.

Attachments

1. Proof of Concept (PoC) File

Rebalancing.sol#L201-L222

  function removePortfolioToken(
    address _token
  ) external onlyAssetManager nonReentrant {
    if (!_isPortfolioToken(_token)) revert ErrorLibrary.NotPortfolioToken();

    // Generate a new token list excluding the token to be removed
    address[] memory currentTokens = _getCurrentTokens();
    uint256 tokensLength = currentTokens.length;
    address[] memory newTokens = new address[](tokensLength - 1);
    uint256 j = 0;
    for (uint256 i; i < tokensLength; i++) {
      address token = currentTokens[i];
      if (token != _token) {
        newTokens[j++] = token;
      }
    }

    portfolio.updateTokenList(newTokens);

    uint256 tokenBalance = IERC20Upgradeable(_token).balanceOf(_vault);
    _tokenRemoval(_token, tokenBalance);
  }

2. Revised Code File (Optional)

skip the balance check for the function call coming from removePortfolioToken inside the _tokenRemoval function.

[langnavina97]

This function is designed to remove the full balance. For partial removal, a separate function is available. @aktech297

[aktech297]

I think the function removes the portfolio token.if any balance is left, that is updated for separate withdrawal by the user

Token removal is the primary purpose when we see the function signature 'removePortfolioToken'

When there are zero balance , the removePortfolioToken call will revert . Pls check

[langnavina97]

After the token has been removed the token balance of the vault should be 0 @aktech297

[aktech297]

I missed that. Let me check

[aktech297]

Hi .

The issue is with token removal, when the vault has zero balance , that means all balances could be withdrawn but the token still exists in the portfolio.

If you see the removePortfolioToken, it check the vault balances and calls the _tokenRemoval function.

[langnavina97]

The full token balance is withdrawn from the portfolio, and the token address is removed from the tokens array. @aktech297

[aktech297]

I think.. I am making some mistake to explain more clear.

Give me some time, I will get back on this

[aktech297]

Hi @langnavina97

Could you please check this .

The function does the following

1. Remove the token from array.
2. Remove the balance.

Rebalancing.sol#L201-L222

  function removePortfolioToken(
    address _token
  ) external onlyAssetManager nonReentrant {
    if (!_isPortfolioToken(_token)) revert ErrorLibrary.NotPortfolioToken();

    // Generate a new token list excluding the token to be removed
    address[] memory currentTokens = _getCurrentTokens();
    uint256 tokensLength = currentTokens.length;
    address[] memory newTokens = new address[](tokensLength - 1);
    uint256 j = 0;
    for (uint256 i; i < tokensLength; i++) {
      address token = currentTokens[i];
      if (token != _token) {
        newTokens[j++] = token;
      }
    }

    portfolio.updateTokenList(newTokens); --@@ token removed.

    uint256 tokenBalance = IERC20Upgradeable(_token).balanceOf(_vault);

    _tokenRemoval(_token, tokenBalance); --@@ balance is removed.
  }

Rebalancing.sol#L291-L293

  function _tokenRemoval(address _token, uint256 _tokenBalance) internal {
    if (_tokenBalance == 0) revert ErrorLibrary.BalanceOfVaultIsZero(); ---->> for zero balance, reverted here.

When token want to be removed, following two cases would happen.

1. the vault. has the balance for token
2. the vault does not have balance for token, but the token present in the portfolio.

For the second case, when the balance of vault for the token is zero, removePortfolioToken will revert inside the _tokenRemoval function.

The fix is simple for this.

if(tokenBalance > 0)  --->> added check here. 
    _tokenRemoval(_token, tokenBalance);

Rebalancing.sol#L201-L222

  function removePortfolioToken(
    address _token
  ) external onlyAssetManager nonReentrant {
    if (!_isPortfolioToken(_token)) revert ErrorLibrary.NotPortfolioToken();

    // Generate a new token list excluding the token to be removed
    address[] memory currentTokens = _getCurrentTokens();
    uint256 tokensLength = currentTokens.length;
    address[] memory newTokens = new address[](tokensLength - 1);
    uint256 j = 0;
    for (uint256 i; i < tokensLength; i++) {
      address token = currentTokens[i];
      if (token != _token) {
        newTokens[j++] = token;
      }
    }

    portfolio.updateTokenList(newTokens);

    uint256 tokenBalance = IERC20Upgradeable(_token).balanceOf(_vault);

    if(tokenBalance > 0)  --->> added check here. 
        _tokenRemoval(_token, tokenBalance);
  }

So, whether the vault has balance or not, the removePortfolioToken should be able to remove the token.

[langnavina97]

Why would you remove a token if there is no balance? @aktech297 The token should not be removed using this function if there is no balance. If an asset manager wants to rebalance the portfolio updateTokens will be used

[aktech297]

Why would you remove a token if there is no balance? @aktech297 The token should not be removed using this function if there is no balance. If an asset manager wants to rebalance the portfolio updateTokens will be used

Ya.. we got it..

we were thinking following scenarios,

1.if asset manager adds token first and for some reason, they want to remove it and add new token.

2. after all the balance of the token is withdrawn from the vault, The token turned into useless (low significant) , the assetmanager would look to remove.

But anyway, if you have any preventive mechanism for this, then all good.