Overview

The initiateLiquidation and finalizeLiquidation functions are designed to handle the liquidation process for users whose health factor falls below a certain threshold. However, there are several issues in the implementation that can lead to incorrect behavior, potential economic losses, and vulnerabilities. Below is a detailed analysis of the issues.

Vulnerability Details

1. Incorrect Health Factor Check:

   • The function checks if healthFactor >= healthFactorLiquidationThreshold to determine if liquidation should be initiated.

   • This logic is backwards. Liquidation should be initiated when the health factor is below the threshold, not above it.

   • Impact: Users who should be liquidated will not be liquidated, leading to under-collateralized positions and potential losses for the protocol.

2. Lack of Access Control:

   • The function does not restrict who can call it. Any external address can initiate liquidation for any user.

   • Impact: Malicious actors could abuse this function to initiate unnecessary liquidations, causing disruption and potential losses for users.

3. Missing Validation for userAddress:

   • The function does not validate whether userAddress is a valid user (e.g., whether userData[userAddress] exists).

   • Impact: Calling the function with an invalid address could lead to unexpected behavior or wasted gas.

• Issue 1: The health factor check is incorrect. It should be healthFactor < healthFactorLiquidationThreshold.

• Issue 2: No access control is applied to restrict who can call this function.

• Issue 3: No validation is performed to ensure userAddress is a valid user.

2. Issues in finalizeLiquidation

Vulnerability Details

1. Incorrect Grace Period Check:

   • The function checks if block.timestamp <= liquidationStartTime[userAddress] + liquidationGracePeriod to determine if the grace period has expired.

   • This logic is backwards. The liquidation should proceed only if the grace period has expired (i.e., block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod).

   • Impact: Liquidations will fail if the grace period has expired, allowing under-collateralized positions to persist.

2. Lack of Validation for userAddress:

   • Similar to initiateLiquidation, the function does not validate whether userAddress is a valid user.

   • Impact: Calling the function with an invalid address could lead to unexpected behavior or wasted gas.

3. Incorrect Debt Burning Logic:

   • The function burns the user's debt using IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex).

   • However, the amountBurned returned from the burn function is not validated to ensure it matches the expected userDebt.

   • Impact: If the burn function does not burn the full debt, the user's scaledDebtBalance will not be updated correctly, leading to incorrect accounting.

4. Incorrect Asset Transfer Logic:

   • The function transfers reserve assets from the msg.sender (Stability Pool) to the reserve.reserveRTokenAddress using safeTransferFrom.

   • However, it does not ensure that the msg.sender has sufficient balance or allowance to cover the transfer.

   • Impact: The transfer could fail, causing the liquidation to revert and leaving the protocol with under-collateralized positions.

5. Incorrect Update of scaledDebtBalance:

   • The function subtracts amountBurned from user.scaledDebtBalance, but it does not ensure that amountBurned is less than or equal to user.scaledDebtBalance.

   • Impact: If amountBurned exceeds user.scaledDebtBalance, the subtraction will underflow, causing the transaction to revert.

• Issue 1: The grace period check is incorrect. It should be block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod.

• Issue 2: No validation is performed to ensure userAddress is a valid user.

• Issue 3: The burn function's output is not validated to ensure the full debt is burned.

• Issue 4: The safeTransferFrom function does not check the msg.sender's balance or allowance.

• Issue 5: The subtraction user.scaledDebtBalance -= amountBurned could underflow if amountBurned > user.scaledDebtBalance.

Recommendations

For initiateLiquidation:

1. Fix Health Factor Check:

   • Change the condition to if (healthFactor >= healthFactorLiquidationThreshold) to if (healthFactor < healthFactorLiquidationThreshold).

2. Add Access Control:

   • Restrict the function to authorized roles (e.g., onlyStabilityPool or onlyLiquidator).

3. Validate userAddress:

   • Add a check to ensure userData[userAddress] exists.

For finalizeLiquidation:

1. Fix Grace Period Check:

   • Change the condition to if (block.timestamp <= liquidationStartTime[userAddress] + liquidationGracePeriod) to if (block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod).

2. Validate userAddress:

   • Add a check to ensure userData[userAddress] exists.

3. Validate Debt Burning:

   • Ensure amountBurned matches userDebt or handle partial burns appropriately.

4. Check msg.sender Balance and Allowance:

   • Add checks to ensure msg.sender has sufficient balance and allowance for the transfer.

5. Prevent Underflow:

   • Add a check to ensure amountBurned <= user.scaledDebtBalance.

Conclusion

The initiateLiquidation and finalizeLiquidation functions contain several critical issues, including incorrect health factor and grace period checks, lack of access control, and improper debt burning logic. By addressing these issues, the liquidation process will function as intended, ensuring the protocol's stability and fairness.