If some tranche (`trancheId`) would be paused, the borrowers who borrowed from the tranche can not repay their debt and there their collaterals will eventually be liquidated

Github username: @0xmuxyz Submission hash (on-chain): 0xb489f535f6724a18adc4ebe918ce1ef0823e704af9ea37d624924d56555f0a0d Severity: medium severity

Description:

Description

Within the LendingPool, the whenTrancheNotPausedAndExists() modifier would be defined in order to check whether or not a given Tranche (trancheId) would not be paused and exists like this:

    modifier whenTrancheNotPausedAndExists(uint64 trancheId) {
        _whenTrancheNotPausedAndExists(trancheId);
        _;
    }

    function _whenTrancheNotPausedAndExists(uint64 trancheId) internal view {
        require(!trancheParams[trancheId].paused && !_everythingPaused, Errors.LP_IS_PAUSED);
        uint64 totalTranches = ILendingPoolConfigurator(_addressesProvider.getLendingPoolConfigurator()).totalTranches();
        require(trancheId<totalTranches, Errors.INVALID_TRANCHE);
    }

The whenTrancheNotPausedAndExists() modifier would be used on the LendingPool#borrow() like this:

    function borrow(
        address asset,
        uint64 trancheId,
        uint256 amount,
        uint16 referralCode,
        address onBehalfOf
    )
        external
        override
        whenTrancheNotPausedAndExists(trancheId) /// @audit
    {
        ...

However, the whenTrancheNotPausedAndExists() modifier would also be used on the LendingPool#repay() like this:

    function repay(
        address asset,
        uint64 trancheId,
        uint256 amount,
        address onBehalfOf
    ) external override whenTrancheNotPausedAndExists(trancheId) returns (uint256) {  /// @audit
        ...

This is problematic. Because if a given Tranche (trancheId) would be paused, the borrowers who borrowed the debt from the given Tranche (trancheId) before the tranche was paused can not repay their debt. As a result, their debt will eventually be liquidated, which means that the borrowers lose their collaterals.

Attack scenario

1: A borrower borrow the amount of debt from a tranche (Let's say trancheId = 1). 2: After that, the tranche admin of the tranche (trancheId = 1) decide to pause their tranche and execute it. 3: The borrower, who borrowed from the tranche (trancheId = 1) at the step1 above, can not repay their debt. Because when the borrower call the LendingPool#repay(), it will be reverted by the whenTrancheNotPausedAndExists() modifier on the the LendingPool#repay(). 4: The debt of the borrower will eventually liquidated and therefore the borrower lose their collateral.

Recommendations

Consider removing the whenTrancheNotPausedAndExists() modifier from the LendingPool#repay() so that the borrowers, who borrowed the debt from the given tranche (trancheId) before the tranche was paused, can repay their debt even if that Tranche (trancheId) would be paused.

The tranche admin does not have the ability to pause tranches, only the emergency admin has that ability, which is an address that is controlled by trusted VMEX admin (see https://github.com/hats-finance/VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/blob/master/packages/contracts/contracts/protocol/lendingpool/LendingPoolConfigurator.sol#L500).

Regardless, even if the tranche is maliciously set to paused, the global admin still has the ability to unpause a tranche given enough evidence that the tranche was maliciously changed.

The "paused" function is used to pause the entire protocol, include repays, in case of critical periods of the protocol. The functionality you mentioned is called "freezing" the reserve, which prevents new deposits and borrows, but still enables withdraws and repays. These are functionality from Aave that we did not change, so this issue is invalid.

hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f