Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dany.armstrong90 - The protocol updates interest rates of collateral wrongly when liquidation. #145

Closed
sherlock-admin4 opened this issue Sep 10, 2024 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Sep 10, 2024

dany.armstrong90

High

The protocol updates interest rates of collateral wrongly when liquidation.

Summary

The protocol does not consider liquidationProtocolFee which transfers to treasury when update interest rates of collateral.

Vulnerability Detail

LiquidationLogic.sol#executeLiquidationCall() is as follows.

  function executeLiquidationCall(
    mapping(address => DataTypes.ReserveData) storage reservesData,
    mapping(uint256 => address) storage reservesList,
    mapping(address => mapping(bytes32 => DataTypes.PositionBalance)) storage balances,
    mapping(address => DataTypes.ReserveSupplies) storage totalSupplies,
    mapping(bytes32 => DataTypes.UserConfigurationMap) storage usersConfig,
    DataTypes.ExecuteLiquidationCallParams memory params
  ) external {
    LiquidationCallLocalVars memory vars;

    DataTypes.ReserveData storage collateralReserve = reservesData[params.collateralAsset];
    DataTypes.ReserveData storage debtReserve = reservesData[params.debtAsset];
    DataTypes.UserConfigurationMap storage userConfig = usersConfig[params.position];
    vars.debtReserveCache = debtReserve.cache(totalSupplies[params.debtAsset]);
    debtReserve.updateState(params.reserveFactor, vars.debtReserveCache);

    ...

    (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()
    );

    ...

174 _burnCollateralTokens(
      collateralReserve, params, vars, balances[params.collateralAsset][params.position], totalSupplies[params.collateralAsset]
    );

    // Transfer fee to treasury if it is non-zero
    if (vars.liquidationProtocolFeeAmount != 0) {
      uint256 liquidityIndex = collateralReserve.getNormalizedIncome();
      uint256 scaledDownLiquidationProtocolFee = vars.liquidationProtocolFeeAmount.rayDiv(liquidityIndex);
      uint256 scaledDownUserBalance = balances[params.collateralAsset][params.position].supplyShares;

      if (scaledDownLiquidationProtocolFee > scaledDownUserBalance) {
        vars.liquidationProtocolFeeAmount = scaledDownUserBalance.rayMul(liquidityIndex);
      }

188   IERC20(params.collateralAsset).safeTransfer(IPool(params.pool).factory().treasury(), vars.liquidationProtocolFeeAmount);
    }

    // Transfers the debt asset being repaid to the aToken, where the liquidity is kept
    IERC20(params.debtAsset).safeTransferFrom(msg.sender, address(params.pool), vars.actualDebtToLiquidate);

    emit PoolEventsLib.LiquidationCall(
      params.collateralAsset, params.debtAsset, params.position, vars.actualDebtToLiquidate, vars.actualCollateralToLiquidate, msg.sender
    );
  }

As we can see above, on L188 it transfers liquidationProtocolFee to treasury.
And _burnCollateralTokens() function called on L174 is as follows.

  function _burnCollateralTokens(
    DataTypes.ReserveData storage collateralReserve,
    DataTypes.ExecuteLiquidationCallParams memory params,
    LiquidationCallLocalVars memory vars,
    DataTypes.PositionBalance storage balances,
    DataTypes.ReserveSupplies storage totalSupplies
  ) internal {
    DataTypes.ReserveCache memory collateralReserveCache = collateralReserve.cache(totalSupplies);
    collateralReserve.updateState(params.reserveFactor, collateralReserveCache);
    collateralReserve.updateInterestRates(
      totalSupplies,
      collateralReserveCache,
      params.collateralAsset,
      IPool(params.pool).getReserveFactor(),
      0,
@>    vars.actualCollateralToLiquidate,
      params.position,
      params.data.interestRateData
    );

    // Burn the equivalent amount of aToken, sending the underlying to the liquidator
    balances.withdrawCollateral(totalSupplies, vars.actualCollateralToLiquidate, collateralReserveCache.nextLiquidityIndex);
    IERC20(params.collateralAsset).safeTransfer(msg.sender, vars.actualCollateralToLiquidate);
  }

As we can see above, the protocol does not consider liquidationProtocolFeeAmount to transfers to treasury.
So interestRates is updated wrongly.

Impact

When liquidation, the protocol updates interestRates wrongly, so protocol interestRates and indexes can be corrupted by liquidation as time goes by.

Code Snippet

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

Tool used

Manual Review

Recommendation

The _burnCollateralTokens() function has to be modified as follows.

  function _burnCollateralTokens(
    DataTypes.ReserveData storage collateralReserve,
    DataTypes.ExecuteLiquidationCallParams memory params,
    LiquidationCallLocalVars memory vars,
    DataTypes.PositionBalance storage balances,
    DataTypes.ReserveSupplies storage totalSupplies
  ) internal {
    DataTypes.ReserveCache memory collateralReserveCache = collateralReserve.cache(totalSupplies);
    collateralReserve.updateState(params.reserveFactor, collateralReserveCache);
    collateralReserve.updateInterestRates(
      totalSupplies,
      collateralReserveCache,
      params.collateralAsset,
      IPool(params.pool).getReserveFactor(),
      0,
-     vars.actualCollateralToLiquidate,
+     vars.actualCollateralToLiquidate + vars.liquidationProtocolFeeAmount,
      params.position,
      params.data.interestRateData
    );

    // Burn the equivalent amount of aToken, sending the underlying to the liquidator
    balances.withdrawCollateral(totalSupplies, vars.actualCollateralToLiquidate, collateralReserveCache.nextLiquidityIndex);
    IERC20(params.collateralAsset).safeTransfer(msg.sender, vars.actualCollateralToLiquidate);
  }

Duplicate of #401

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A High severity issue. labels Sep 20, 2024
@sherlock-admin3 sherlock-admin3 changed the title Decent Walnut Squirrel - The protocol updates interest rates of collateral wrongly when liquidation. dany.armstrong90 - The protocol updates interest rates of collateral wrongly when liquidation. Oct 3, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Oct 3, 2024
@WangSecurity WangSecurity added Medium A Medium severity issue. and removed High A High severity issue. labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

3 participants