hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

LP token lending protocol
MIT License
2 stars 1 forks source link

Impossible to start or stop staking #32

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @bahurum Submission hash (on-chain): 0xf2bd7d7647b88e237cabb4bc4e6cdaf05532fd801331ec7000be3d8c7ac2a2d6 Severity: medium severity

Description:

Description

ExternalRewardDistributor's functions beginStakingReward() and removeStakingReward() will revert because they use the totalSupply() of the aToken instead of the underlying balance of the aToken as amount of underlying to receive or send to the stakingContract,

Vulnerability details:

In ExternalRewardDistributor.beginStakingReward() all the underlying which is held in the aToken contract should be staked.

Instead the amount is set to IERC20(aToken).totalSupply() which is always larger than the amount of underlying available because a part of the underlying will be lent out. This will make IERC20(underlying).safeTransferFrom() revert.

There is a case in which beginStakingReward() will not revert, and that is when nobody has borrowed yet the underlying from that aToken.

This could be the case if the admins of the tranche activate the staking rewards at launch.

In this case beginStakingReward() will not revert, but now for the same reason as above removeStakingReward() will always fail, so staking cannot be stopped.

Recommendation

Use the current underlying balance of the aToken when staking and the current underlying balance of the staking contract when unstaking.

  function beginStakingReward(

    ...

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

    ...

  function removeStakingReward(address aToken) external onlyManager {
    address underlying = IAToken(aToken).UNDERLYING_ASSET_ADDRESS();
    uint64 trancheId = IAToken(aToken)._tranche();

-   uint256 amount = IERC20(aToken).totalSupply();
+   address underlying = IAToken(aToken).UNDERLYING_ASSET_ADDRESS();
+   uint256 amount = IERC20(underlying).balanceOf(stakingData[underlying][trancheId]);
    if(amount!=0){
      IStakingRewards(stakingData[underlying][trancheId]).withdraw(amount);
      IERC20(underlying).safeTransfer(aToken, amount);
    }
ksyao2002 commented 1 year ago

The underlying will not be lent out. Any token with staking rewards will not be borrowable, as enforced by this check https://github.com/hats-finance/VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/blob/fb396a3fa412e643de7d8a1fd8a0268ab3a2f993/packages/contracts/contracts/protocol/incentives/ExternalRewardDistributor.sol#L58