hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

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

Staking activation will block borrowing #33

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: high severity

Description:

Description

Activating staking rewards will transfer all the underlying inside an aToken to the staking contract. LendingPool.borrow() will then revert because IAToken.transferUnderlyingTo() attempts to transfer the collateral amount directly from the aToken to the borrower.

Vulnerability details:

  1. manager activates staking for a given aToken by calling ExternalRewardsDistributor.beginStakingReward(). All underlying inside the aToken is staked into the stakingContract.
  2. A user calls LendingPool.borrow(). The amount to be borrowed should be transfered by the call to IAToken(vars.aTokenAddress).transferUnderlyingTo():
    function transferUnderlyingTo(address target, uint256 amount)
        external
        override
        onlyLendingPool
        returns (uint256)
    {
        IERC20(_underlyingAsset).safeTransfer(target, amount);
        return amount;
    }

    This will fail because the underlying balance of the aToken is zero.

So the call to LendingPool.borrow() reverts and the user cannot borrow.

Note that, as shown in a separate finding (called :"Impossible to start or stop staking"), if the staking rewards are activated at tranche deployment, then it won't be possible to deactivate staking for the aToken so borrow() will be permanently broken for the underlying of that aToken.

Recommendation

The direct transfer of funds to the borrower should be done from the staking contract when staking is active. Consider adding to AToken.transferUnderlyingTo() a call to withdraw the amount to borrow from the staking contract.

function transferUnderlyingTo(address target, uint256 amount)
    external
    override
    onlyLendingPool
    returns (uint256)
{
    IERC20(_underlyingAsset).safeTransfer(target, amount);
+   if (address(_getIncentivesController()) != address(0)) {
+       _getIncentivesController().handleAction(
+           target,
+           totalSupply(),
+           0,
+           amount,
+           DistributionTypes.Action.WITHDRAW
+       );
+   }
    return amount;
}
bahurum commented 1 year ago

There is an error in the recommendation: IERC20(_underlyingAsset).safeTransfer(target, amount) should come after the if block

ksyao2002 commented 1 year ago

Borrowing is not enabled for tokens with external rewards staking, as mentioned previously. https://github.com/hats-finance/VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/issues/32