Core Contracts

Summary

The functionality initiateLiquidation within LendingPool.sol allows any user to initiate the liquidation process if their health factor is below the threshold. The health factor is calculated via calculateHealthFactor on the userAddress we pass into the function. Ultimately, the user's collateral value must stay above the threshold of their borrowed amount in order to avoid liquidation.

Vulnerability Details

When executing the initiateLiquidation function, there are two problems:

• Anybody can call the function with any address, meaning we can initiate liquidation for other users which is not the intended usage as it is counter intuitive. It basically turns the liquidation process into a battlefield amongst users, at least for those who are aware.

• When labelling the user as liquidated, we do not update the UserData struct for the user we initialise on L453 within LendingPool.sol:

And whilst these two variables are used, this does create a mismatch for the future of the protocol when future contracts may utilise the state of the user via UserData.

The struct itself contains the following data regarding the user:

Clearly the intended usage within the code was to update the mapping of the user and the respective UserData fields, because we initialise the user data yet make no use of it whatsoever.

To further prove the intended usage, we utilise the UserData user initialisation in multiple instances, such as depositNFT() in order to check whether the user's state has already deposited an NFT with the same tokenId, or when withdrawing an NFT to see if we deposited the tokenId we are trying to withdraw. There are also many other instances of the usage of UserData in respect of the user. It is rather problematic to mix states like the user's userData, yet have a separate global mapping for the same variable within the struct.

Impact

When any future code is implemented which relies on a user's initialised fields within userData, then we have wrongful accounting as technically the direct mapping isUnderLiquidation[userAddress] is set to true, yet the actual UserData field contains a false value for underLiquidation. Unfortunately, getAllUserData is also impacted and would return the wrong isUnderLiquidation value.

Tools Used

Manual review

Recommendations

Update the states of the user during a process like liquidation for all mappings and user data where relevant and as intended