Skip to content

Latest commit

 

History

History
70 lines (54 loc) · 3.56 KB

065.md

File metadata and controls

70 lines (54 loc) · 3.56 KB

Orbiting Sangria Porpoise

Medium

closeLeverageStrategy() is not allowed during vault's paused state but repayBorrow() is

Description

It's the protocol's prerogative to take the design call whether repayment of borrows should be allowed or not when the vault is paused. The protocol could either say:

  1. It's allowed because we want borrowers to be able to close their risk exposure and threat of liquidation OR
  2. It's not allowed because protocols are paused in emergency situations when a high risk vulnerability has been uncovered and hence we do not want any balance changes to happen due to user transactions right now.

It's interesting to observe however that:

  1. closeLeverageStrategy() is not allowed during a paused state. It internally calls vault.repayLeverage(true) and repayLeverage is protected by the whenNotPaused modifier.
  2. The regular repayBorrow() is allowed to operate even when the protocol is paused, showing the inconsistency in code implementation. The PoC section shows a test demonstrating this.

Impact

Irrespective of whether the protocol's design decision is the first or the second, the inconsistency in the two repay functions causes either the potential loss of funds to the borrowers, or the risk of protocol exploit during the paused state.

Proof of Concept

Add the test inside Vault.t.sol and see repayBorrow() pass even when the vault is paused:

    function test_repayDuringPause() public {
        uint borrowAmount = 0.1 ether;
        deal({token: address(rEth), to: userA, give: 10000 ether});
        vm.prank(deployer);
        comptroller._setCollateralFactor(cNuma, 1 ether);
        
        // First approve and enter the NUMA market
        vm.startPrank(userA);
        address[] memory t = new address[](1);
        t[0] = address(cNuma);
        comptroller.enterMarkets(t);

        // Buy some NUMA
        rEth.approve(address(vault), 10 ether);
        uint numas = vault.buy(10 ether, 0, userA);

        // Deposit enough NUMA as collateral
        numa.approve(address(cNuma), numas);
        cNuma.mint(numas);

        // Borrow rEth
        uint balanceBefore = rEth.balanceOf(userA);
        cReth.borrow(borrowAmount);
        assertEq(rEth.balanceOf(userA) - balanceBefore, borrowAmount, "Borrow failed");

        // Get current borrow balance 
        uint borrowBalance = cReth.borrowBalanceCurrent(userA);
        assertGt(borrowBalance, 0, "No borrow balance");
        vm.stopPrank();

        // Pause the vault
        vm.prank(deployer);
        vault.pause();

        // Try to repay while paused
        vm.startPrank(userA);
        rEth.approve(address(cReth), borrowBalance);
        cReth.repayBorrow(borrowBalance);  // <----------------- @audit : does not revert !

        vm.stopPrank();
    }

Mitigation

Either both functions' call shouldn't be allowed, or both should be.

Note

It might be the case that closeLeverageStrategy() should perhaps be allowed even in paused state as per the following comment present inside ComptrollerStorage.sol:

Actions which allow users to remove their own assets cannot be paused.