Core Contracts

Vulnerability Details

When a user adds more collateral his health factor improves. This can lead to a liquidatable user to suddenly not to be liquidatabale due to having more value as collateral.

However the code does not account for this and if someone has been called at LendingPool::initiateLiquidation() and then adds more collateral, the user will still be marked as isUnderLiquidation[userAddress] = true;.

Impact

This can lead to a user thinking he is healthy because he deposited more collateral but in reality the system will mark him as not. This leads to him being able to be liquidated from StabilityPool::liquidateBorrower().

As you can see here there are no checks for health on the StabilityPool::liquidateBorrower() function. Neither in the LendingPool::finalizeLiquidation(), which only checks if isUnderLiquidation[userAddress] == true, here.

The reason on why there are no checks of health factor on this processes is because the health factor was already checked on LendingPool::initiateLiquidation(), here.

But this assumption is wrong, as the health factor can change during the time a liquidation process is initialized to finalized. For example if the user directly calls LendingPool::depositNFT() as explained. Adding more collateral and improving his health. See here to see that there are no re-checks of helath factor on depositing new collareral.

Recommendations

When adding collateral, in the form of NFTs on this system, check the health factor, and if healthy mark the liquidatable related states to not liquidatable. These are: