000000 - Shares of a user will be wrong after a liquidation

[sherlock-admin4]

000000

High

Shares of a user will be wrong after a liquidation

Summary

Shares of a user will be wrong after a liquidation

Vulnerability Detail

Upon a liquidation, LiquidationLogic::executeLiquidationCall() is called. There, we calculate the collateral the user will get and the fees the protocol gets:

(vars.actualCollateralToLiquidate, vars.actualDebtToLiquidate, vars.liquidationProtocolFeeAmount) =
    _calculateAvailableCollateralToLiquidate(
      collateralReserve,
      vars.debtReserveCache,
      vars.actualDebtToLiquidate,
      vars.userCollateralBalance,
      vars.liquidationBonus,
      IPool(params.pool).getAssetPrice(params.collateralAsset),
      IPool(params.pool).getAssetPrice(params.debtAsset),
      IPool(params.pool).factory().liquidationProtocolFeePercentage()
    );

Note that vars.actualCollateralToLiquidate + vars.liquidationProtocolFeeAmount is the total collateral that will be taken from the user, the actual collateral to liquidate variable does not include the fees. Then, we call LiquidationLogic::_burnCollateralTokens() where we call the following 2 functions:

balances.withdrawCollateral(totalSupplies, vars.actualCollateralToLiquidate, collateralReserveCache.nextLiquidityIndex); IERC20(params.collateralAsset).safeTransfer(msg.sender, vars.actualCollateralToLiquidate);

As seen, we call balances.withdrawCollateral() (which simply deducts the collateral shares from the total supply and from the user's shares) and then transfer that amount to the liquidator. The issue is that this amount does not include the fees as mentioned earlier, thus whenever, for example, full liquidation occurs, the user will still have some shares leftover as the actual amount of collateral we are taking from the user is vars.actualCollateralToLiquidate + vars.liquidationProtocolFeeAmount, not just vars.actualCollateralToLiquidate. Then, upon the transfer of collateral to the protocol as fees, we don't burn that amount of shares from the user being liquidated which is the way Aave does it.

Impact

Shares of a user will be wrong after a liquidation

Code Snippet

https://github.com/sherlock-audit/2024-06-new-scope/blob/c8300e73f4d751796daad3dadbae4d11072b3d79/zerolend-one/contracts/core/pool/logic/LiquidationLogic.sol#L228

Tool used

Manual Review

Recommendation

Include the fees the protocol gets when deducting from the user shares or burn shares from the user upon the transfer of collateral to the protocol, the way Aave does it

Duplicate of #228