Don't Allow portfoliotoken share transfers and portfolio token removals during pause state

[hats-bug-reporter[bot]]

Github username: @burhankhaja Twitter username: imaybeghost Submission hash (on-chain): 0xa56efc689f4e99d556a9e6b1aa93b0e4897afad1796ef110dcc03c202373e54e Severity: medium

Description: Description\ During the pause state, the protocol prevents user deposits and withdrawls (minting and burning) but it doesn't restrict token transfers via transfer(). (PortfolioToken.sol)

Similarly, for the asset manager in Rebalancing contract, the protocol restricts updateWeights() && updateTokens() but doesn't restrict token removal functions. Which kinda break the purpose of pausing mechanisms.

Since Pausing a smart contract is typically done to minimize the impact of exploits on user funds, mitigate security issues, and allow for flexible debugging.

Attack Scenario\ Since this is business logic flaw:

• Portfoliotoken shares can be easily transfered while the protocol is paused via ERC20 transfer() function.
• AssetManager can still Remove portfolio and non-portfolio tokens even during the protocol is paused, which may affect business operations of the protocol

Recommendation\ .

• Check if protocol is paused in _beforeTokenTransfer()

  
  function _beforeTokenTransfer(
  address from,
  address to,
  uint256 amount
  ) internal override {
  super._beforeTokenTransfer(from, to, amount);
  if (from == address(0) || to == address(0)) {
    return;
  }
  if (
    !(assetManagementConfig().transferableToPublic() ||
      (assetManagementConfig().transferable() &&
        assetManagementConfig().whitelistedUsers(to)))
  ) {
    revert ErrorLibrary.Transferprohibited();
  }
  _checkCoolDownPeriod(from);

• if (IProtocolConfig(protocolConfig).isProtocolPaused()) revert ErrorLibrary.ProtocolIsPaused();
  }

• Enfore similar check in _tokenRemoval()

  
  function _tokenRemoval(address _token, uint256 _tokenBalance) internal {

• if (IProtocolConfig(protocolConfig).isProtocolPaused()) revert ErrorLibrary.ProtocolIsPaused();
  if (_tokenBalance == 0) revert ErrorLibrary.BalanceOfVaultIsZero();

  // Snapshot for record-keeping before removing the token uint256 currentId = tokenExclusionManager.snapshot();

  // Record the removal details in the token exclusion manager tokenExclusionManager.setTokenAndSupplyRecord( currentId - 1, _tokenBalance, _token, portfolio.totalSupply() );

  // Transfer the token balance from the vault to the token exclusion manager portfolio.pullFromVault( _token, _tokenBalance, address(tokenExclusionManager) );

  // Log the token removal event emit PortfolioTokenRemoved(_token, _tokenBalance, currentId - 1); }

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[langnavina97]

For transfers, it's not required. For removing tokens, we've added it to our findings in the sheet yesterday. @burhankhaja

[burhankhaja]

Hey @langnavina97 i was talking to Hats finance security team and they recommended me to first resolve this issue here, if not then they recommended to escalate it later

Since in audit contest, general rule applies that you can't update the codebase or publicly known issues while the contest is live.