Centralization Risk, sweep tokens doesn't protect tokenized deposits

[hats-bug-reporter[bot]]

Github username: @GalloDaSballo Submission hash (on-chain): 0x39e6bdd72c6d292861d39347759d05434e7dfa37f4bbf1c8f6089f093fe6de87 Severity: low severity

Description: Description\ RescueRewardTokens doesn't prevent sweeping away the underlying deposits

Any token can be reward, some / most new gauges (e.g. Convex / Aura) are ERC4626 tokens meaning they can be sweeped away as well

Attack Scenario\ For whatever reason, either via a Multisig Compromise, or a Governance takeover, the manager decides to sweep away the Tokenized Deposits, breaking the protocol functionality and stealing deposits

Code Snippet

https://github.com/hats-finance/VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/blob/f14637deda71be440d1cfbf31cc12ede69ff3a6b/packages/contracts/contracts/protocol/incentives/ExternalRewardDistributor.sol#L150-L151

    reward.safeTransfer(receiver, reward.balanceOf(address(this)));

POC reward.safeTransfer(AURA_DEPOSIT, AMT, {"from": manager})

Revised Code File

  function rescueRewardTokens(IERC20 reward, address receiver) external onlyManager {
    require(!protected(reward)); // Check if token is protected, make gauges protected when staking starts
    reward.safeTransfer(receiver, reward.balanceOf(address(this)));
  }

[ksyao2002]

Thanks for the report. Underlying tokens that are deposited are never owned by the IncentivesController. It either immediately stakes the tokens or unstakes them and sends them back to the aToken contract. Thus, such a risk would not pose a problem.