c4-bot-2 commented 2 months ago

Lines of code

Vulnerability details

C4 issue

M-11: Lack of safety buffer in _checkLoanIsHealthy

Comments

Revert does not have a safety buffer for borrowed assets. Without a safety buffer, a borrower can borrow the maximum amount of funds equal to their collateral and if any minor price changes occurs in the market, the borrower runs the risk of being liquidated. This is encapsulated in the _requireLoanIsHealthy() function which maintains the following formula:

isHealthy = collateralValue >= debt;

As can be seen, there is no buffer between borrowed debt and collateral.

Mitigation

PR #17

This mitigation involves two fixes:

The first change updates the _checkLoanIsHealthy function to accept an additional argument called withBuffer. This param when set to true will re-calculate the collateral value to the following formula: collateralValue * BORROW_SAFETY_BUFFER_X32 / Q32. BORROW_SAFETY_BUFFER_X32 is also a new constant introduced in this fix and is set to 95% Q32. This formula reduces the collateral value by 5% and ensures that debt can never exceed 95% of the collateral. This adds a buffer if the debt or collateral values shift due to minor market movements.

The second change involves updating each call to _checkLoanIsHealthy() via passing in a new parameter called withBuffer. If set to true, this argument will calculate the collateral value with a buffer. If false, the collateral value will be calculated without a buffer. Below is a detailed list of each function that calls _checkLoanIsHealthy() and it's withBuffer setting:

withBuffer is set to true:
- V3Vault.borrow(): Setting buffer to true is necessary as any borrower wanting to borrow funds will need to have a buffer in place to ensure that a minor market price change does not cause the loan to be immediately liquidated. Note that when V3Vault.liquidate() is called, the withBuffer value is set to false. This ensures that a loan can't be immediately liquidated.
- V3Vault.decreaseLiquidityAndCollect(): This function reduces the collateral for a given loan. Setting buffer to true here ensures that the borrower does not withdraw so much collateral that they are below the buffer zone. This prevents the borrower from experiencing a sudden liquidation due to minor market swings.
withBuffer is set to false:
- V3Vault.liquidate(): Part of liquidating a position is ensuring that the NFT position is unhealthy and can be liquidated. By checking that the collateral exceeds the debt without a buffer, Revert can accurately determine if the NFT position is liquidatable.
- V3Vault.loanInfo(): This is a view function utilized by FlashLoanLiquidator. Since FlashLoanLiquidator may liquidate a loan, the collateral should be calculated without a buffer.
- V3Vault.transform(): Transform applies changes to a NFT position. It is critical that the position is checked to see if it's still healthy without a buffer. Otherwise, the NFT position is unhealthy and transform() should revert.

To sum up, buffers should only be used when the loan's borrow or collateral changes. This prevents surprise liquidations. A buffer should NOT be used when calculating whether or not a loan should be liquidated.

Based on these changes, a buffer is correctly applied protecting borrowers from surprise liquidations.