beginStakingReward doesn't work after any interest is accrued

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

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

 function beginStakingReward(
    address aToken,
    address stakingContract
  ) public onlyManager { //if tranches want to activate they need to talk to us first
    address assetMappings = addressesProvider.getAssetMappings();
    address underlying = IAToken(aToken).UNDERLYING_ASSET_ADDRESS();
    uint64 trancheId = IAToken(aToken)._tranche();

    require(!stakingExists(aToken), "Cannot add staking reward for a token that already has staking");
    require(!IAssetMappings(assetMappings).getAssetBorrowable(underlying), "Underlying cannot be borrowable for external rewards");

    stakingData[underlying][trancheId] = stakingContract;
    IERC20(underlying).approve(stakingContract, type(uint).max);

    //transfer all aToken's underlying to this contract and stake it
    uint256 amount = IERC20(aToken).totalSupply();
    if(amount!=0){
      IERC20(underlying).safeTransferFrom(aToken, address(this), amount);
      IStakingRewards(stakingContract).stake(amount);
    }

    emit RewardConfigured(aToken, stakingContract, amount);
  }

Description\

beginStakingReward will deposit all tokens into the staking contract

To do so it will use totalSupply which, if any interest was accrued, will pull more than all available funds, causing a revert.

Attack Scenario\

Due to

    uint256 amount = IERC20(aToken).totalSupply();
    if(amount!=0){
      IERC20(underlying).safeTransferFrom(aToken, address(this), amount);
      IStakingRewards(stakingContract).stake(amount);
    }

Using totalSupply instead of the underlying balance, the transfer will rever in most cases

This will cause all tokens to be stuck due to totalSupply increasing

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

   * @dev Removes all liquidity from the staking contract and sends back to the atoken. Subsequent calls to handleAction doesn't call onDeposit, etc
   * @param aToken The address of the aToken that wants to exit the staking contract
   **/
  function removeStakingReward(address aToken) external onlyManager {
    address underlying = IAToken(aToken).UNDERLYING_ASSET_ADDRESS();
    uint64 trancheId = IAToken(aToken)._tranche();

    uint256 amount = IERC20(aToken).totalSupply();
    if(amount!=0){
      IStakingRewards(stakingData[underlying][trancheId]).withdraw(amount);
      IERC20(underlying).safeTransfer(aToken, amount);
    }
    stakingData[underlying][trancheId] = address(0);

    //event
    emit StakingRemoved(aToken);
  }

This can create a scenario where older tranches have to be deprecated as it's not possible to call init on a reserve that was already created:

    function init(
        DataTypes.ReserveData storage reserve,
        address aTokenAddress,
        address variableDebtTokenAddress,
        address interestRateStrategyAddress
    ) external {
        require(
            reserve.aTokenAddress == address(0),
            Errors.RL_RESERVE_ALREADY_INITIALIZED
        );

While it may be acceptable to force a migration for newer tranches, it may be best to rewrite the code to allow such state

Otherwise tokens for which new staking contracts are added, would be in a "limbo" state, where older tranches are incompatible with staking, while newer ones are

Recommendation

To keep the protocol competitive, a specific migration scenario should be thought out

Alternatively, the contracts could be changed to handle that edge case, or at the very least, refactored to use the balance available instead of the totalSupply

hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

beginStakingReward doesn't work after any interest is accrued #57