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

joshuajee - Borrowers can make their position unprofitable to liquidated by using too many collateral tokens. #231

Open
sherlock-admin3 opened this issue Sep 10, 2024 · 41 comments
Labels
Escalated This issue contains a pending escalation Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Sep 10, 2024

joshuajee

High

Borrowers can make their position unprofitable to liquidated by using too many collateral tokens.

Summary

The maximum number of reserve tokens that a pool can have is 128 (This to prevent out of gas error).
During liquidation all 128 assets are looped through adding to the total gas usage,
a malicious user can exploit this by providing collateral worth 500 USD in the 128 reserve tokens, so they supply collateral of about 3.9 USD in each.
after that, they borrow about 400 of the tokens they want.

Root Cause

The root cause of this bug lies in the fact that users can supply any amount of collateral even 1 wei and still take loans with it.
The more the numbers of collaterals they have in their basket the more the gas usage during liquidation.

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

  function calculateUserAccountData(
    mapping(address => mapping(bytes32 => DataTypes.PositionBalance)) storage _balances,
    mapping(address => DataTypes.ReserveData) storage reservesData,
    mapping(uint256 => address) storage reservesList,
    DataTypes.CalculateUserAccountDataParams memory params
  ) internal view returns (uint256, uint256, uint256, uint256, uint256, bool) {

    //@query what happens when i supply and withdraw
    if (params.userConfig.isEmpty()) {
      return (0, 0, 0, 0, type(uint256).max, false);
    }

    CalculateUserAccountDataVars memory vars;
    uint256 reservesCount = IPool(params.pool).getReservesCount();
@-> while (vars.i < reservesCount) {
      if (!params.userConfig.isUsingAsCollateralOrBorrowing(vars.i)) {
        unchecked {
          ++vars.i;
        }
        continue;
      }
    ..

For example
ETH = 2271 USD
if they borrow with the whole 128 assets as collateral the gas used during liquidation will be 1,005,205 that's about 2.28 USD in gas cost.
Given that the collateral for the single asset that the liquidator wants to liquidate is 3.9 USD, the liquidation is not really worth it as the liquidation bonus cannot cover the gas cost.

The total cost of liquidating the whole position is (1,005,205 * 128 / 2) = 64,333,120 i.e 147 USD

Internal pre-conditions

A pool with 128 reserve tokens

External pre-conditions

No response

Attack Path

Assuming that the average LTV is 0.8

  1. Attacker supplies 3.9 USD to all 128 reserves which is about 500 USD
  2. Attacker borrows 400 USD.
  3. Heath factor falls below 1.
  4. The gas cost of liquidating just one collateral is 2.28 USD and the gain is (3.9 * 1.05 - 3.9) = 0.195 USD
  5. Because the gas cost in liquidating the position is far greater than the liquidation bonus they will not liquidate the position.

Impact

  1. The position will not be liquidated on time.
  2. Many positions will go insolvent without liquidators to close them.
  3. Liquidity providers will lose money.

PoC

Add the following test to the forge/core/pool folder and run the test.

This is the output.

Gas Used : 1005198
HF before : 916085486657142857 35000000000
HF After : 916743203363504031 34700037495

We can see the current gas cost is 1005198.

// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.19;

import './PoolSetup.sol';

import {ReserveConfiguration} from './../../../../contracts/core/pool/configuration/ReserveConfiguration.sol';

import {UserConfiguration} from './../../../../contracts/core/pool/configuration/UserConfiguration.sol';

contract LiquidationGGPOC is PoolSetup {
  using UserConfiguration for DataTypes.UserConfigurationMap;
  using ReserveConfiguration for DataTypes.ReserveConfigurationMap;


  MintableERC20[] tokens;
  MockV3Aggregator[] oracles;

  uint reserveCount = 128;

  address alice = makeAddr("alice");
  address bob = makeAddr("bob");
  uint256 mintAmount =  50000 ether;
  uint256 mintAmountB = 50000 ether;
  uint256 supplyAmountA = 500 ether;
  uint256 supplyAmountB = 750 ether;
  uint256 borrowAmountB = 350 ether;

  function setUp() public {
    poolImplementation = new Pool();

    poolFactory = new PoolFactory(address(poolImplementation));
    configurator = new PoolConfigurator(address(poolFactory));

    poolFactory.setConfigurator(address(configurator));

    for (uint i = 0; i < reserveCount - 1; i++) {
        tokens.push(new MintableERC20('TOKEN A', 'TOKENA'));
        oracles.push(new MockV3Aggregator(8, 1e8));
    }

    tokens.push(new MintableERC20('WETH9', 'WETH9'));
    oracles.push(new MockV3Aggregator(6, 3000 * 1e8));

    irStrategy = new DefaultReserveInterestRateStrategy(47 * 1e25, 0, 7 * 1e25, 30 * 1e25);

    vm.label(address(poolFactory), 'PoolFactory');
    vm.label(address(irStrategy), 'irStrategy');
    vm.label(address(configurator), 'configurator');

    poolFactory.createPool(_poolInitParams());
    IPool poolAddr = poolFactory.pools(0);
    pool = IPool(address(poolAddr));

    pos = keccak256(abi.encodePacked(alice, 'index', uint256(0)));
  }

  function testLiquidationSimple() external {

    _mintAndApprove(alice, tokens[reserveCount - 1], mintAmount, address(pool));

    //Token index 0 is the collateral
    //Token index 1 is the borrowed token
    for (uint i = 0; i < reserveCount - 1; i++) {
        //Don't mint the borrowed token for alice
        _mintAndApprove(alice, tokens[i], mintAmount, address(pool));
        vm.prank(alice);
        pool.supplySimple(address(tokens[i]), alice, (supplyAmountA / (reserveCount - 1)), 0);
        //console.log("S: ", supplyAmountA / (reserveCount - 1));
    }

    //Mint the borrowed token for bob
    _mintAndApprove(bob, tokens[1], mintAmount, address(pool));
    vm.prank(bob);
    pool.supplySimple(address(tokens[1]), bob, supplyAmountB, 0); // 750 tokenB bob supply

    vm.startPrank(alice);

    pool.supplySimple(address(tokens[reserveCount - 1]), alice, 1, 0); // send 1 weth tokenA alice supply
    pool.borrowSimple(address(tokens[1]), alice, borrowAmountB, 0); // 100 tokenB alice borrow
    vm.stopPrank();

    for (uint i = 0; i < reserveCount - 1; i++) {
        if (i == 1) continue;
        oracles[i].updateAnswer(8e7);
    }

    (,uint debtBefore,,,,uint healthFactorBefore) = pool.getUserAccountData(alice, 0);

    uint initialGas = gasleft();
    vm.prank(bob);
    pool.liquidateSimple(address(tokens[0]), address(tokens[1]), pos, 200 ether);
    console.log("Gas Used :", initialGas - gasleft());


    (,uint debtAfer,,,, uint healthFactorAfter) = pool.getUserAccountData(alice, 0);

    console.log("HF before :", healthFactorBefore, debtBefore);
    console.log("HF After  :", healthFactorAfter, debtAfer);

  }

function _poolInitParams() internal view returns (DataTypes.InitPoolParams memory p) {

    (
    address[] memory assets, 
    address[] memory rateStrategyAddresses, 
    address[] memory sources, 
    DataTypes.InitReserveConfig[] memory configurationLocal 
    ) = _generateAddresses();

    address[] memory admins = new address[](1);
    admins[0] = address(this);

    p = DataTypes.InitPoolParams({
      proxyAdmin: address(this),
      revokeProxy: false,
      admins: admins,
      emergencyAdmins: new address[](0),
      riskAdmins: new address[](0),
      hook: address(0),
      assets: assets,
      rateStrategyAddresses: rateStrategyAddresses,
      sources: sources,
      configurations: configurationLocal
    });
  }


  function _generateAddresses()
    internal view returns (address[] memory, address[] memory, address[] memory, DataTypes.InitReserveConfig[] memory) {
    address[] memory assets = new address[](reserveCount);
    address[] memory rateStrategyAddresses = new address[](reserveCount);
    address[] memory sources = new address[](reserveCount);
    DataTypes.InitReserveConfig memory config = _basicConfig();
    DataTypes.InitReserveConfig[] memory configurationLocal = new DataTypes.InitReserveConfig[](reserveCount);
    for (uint i = 0; i < reserveCount; i++) {
        assets[i] = address(tokens[i]);
        rateStrategyAddresses[i] = address(irStrategy);
        sources[i] = address(oracles[i]);
        configurationLocal[i] = config;
    }
    return (assets, rateStrategyAddresses, sources, configurationLocal);
  }

}

Mitigation

  1. Enforce a minimum supply amount for an asset to be used as collateral.
  2. Limit the total number of active collateral for a position to 10 tokens.
@github-actions github-actions bot added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. labels Sep 20, 2024
@sherlock-admin3
Copy link
Contributor Author

1 comment(s) were left on this issue during the judging contest.

Honour commented:

It's not practical for a pool to have that many tokens. Deploying such a pool itself would be gas intensive for the owner (especially on mainet). Also the 128 limit is not neccessarily to prevent out of gas errors but because collateral/debt info is stored in a 256-bit word for gas efficiency

@nevillehuang
Copy link
Collaborator

nevillehuang commented Oct 2, 2024

This is a very interesting finding, will leave open for escalation discussion, especially when pool creation is permisionless so is subject to various configuration

@sherlock-admin3 sherlock-admin3 changed the title Shiny Seaweed Mongoose - Borrowers can make their position unprofitable to liquidated by using too many collateral tokens. joshuajee - Borrowers can make their position unprofitable to liquidated by using too many collateral tokens. Oct 3, 2024
@sherlock-admin3 sherlock-admin3 added Reward A payout will be made for this issue and removed Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Oct 3, 2024
@DemoreXTess
Copy link

Escalate

This issue is valid, but it should be classified as low or informational severity. The issue largely depends on the pool configurations, which are designed by the pool admin. To mitigate this problem, the admin should configure such pools with a high liquidation bonus rate. The report inaccurately states that liquidation would not occur on time. This does not make the position unliquidatable; rather, it discourages liquidators from liquidating the position due to the gas costs.

It is also important to note that liquidation does not always guarantee a profit in lending protocols, mainly due to gas costs. In Sherlock, gas-related issues are only considered significant when they lead to time-sensitive DoS problems, such as out-of-gas errors in critical functions. The loss in this situation is entirely due to gas fees, which cannot be classified as high or medium severity.

Finally, one of the core features of ZeroLend is its permissionless pools, which can hold multiple tokens. Limiting the number of tokens is not a reasonable solution to this problem.

@sherlock-admin3
Copy link
Contributor Author

Escalate

This issue is valid, but it should be classified as low or informational severity. The issue largely depends on the pool configurations, which are designed by the pool admin. To mitigate this problem, the admin should configure such pools with a high liquidation bonus rate. The report inaccurately states that liquidation would not occur on time. This does not make the position unliquidatable; rather, it discourages liquidators from liquidating the position due to the gas costs.

It is also important to note that liquidation does not always guarantee a profit in lending protocols, mainly due to gas costs. In Sherlock, gas-related issues are only considered significant when they lead to time-sensitive DoS problems, such as out-of-gas errors in critical functions. The loss in this situation is entirely due to gas fees, which cannot be classified as high or medium severity.

Finally, one of the core features of ZeroLend is its permissionless pools, which can hold multiple tokens. Limiting the number of tokens is not a reasonable solution to this problem.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin4 sherlock-admin4 added the Escalated This issue contains a pending escalation label Oct 3, 2024
@Joshuajee
Copy link

Liquidation is a very time-sensitive function in DEFI, when a position starts losing they have to be liquidated as soon as possible. if liquidation is not profitable liquidators won't close such positions. This will lead to insolvency, this risk is too much to ignore. As for the recommendation it is left for the protocol to fix the issue as they see fit. Nevertheless, it doesn't make sense to have more than 10 collateral to back a single position.

Liquidation Incentive is a core feature of borrowing and lending protocol. Lack of Liquidation incentive is a valid High when it comes to DEFI. Mind you there is no mechanism to handle bad debt in zero-lend. So when bad debt occurs it will cause chaos in that pool (Race to remove Liquidity).

@0xjuaan
Copy link

0xjuaan commented Oct 6, 2024

Admins are assumed to know the consequence of their actions, so admins will not add so many collateral tokens such that liquidations are unprofitable

@Joshuajee
Copy link

Joshuajee commented Oct 6, 2024

@0xjuaan

Admins are assumed to know the consequence of their actions, so admins will not add so many collateral tokens such that liquidations are unprofitable

The protocol is permissionless, which means that anyone can create pools, the contract allows up to 128 assets per pool, and there is no assumption that users will not create such large pools. The limit is supposed to prevent something like this but it doesn't and you just came up with that assumption.

@cvetanovv
Copy link

This issue is quite interesting and falls on the borderline between Medium and Low severity.

In theory, this vulnerability is technically possible because ZeroLend is permissionless, meaning that anyone can create a pool. This would allow a user to create a pool with 128 tokens, leading to the gas inefficiency issue described.

However, there is a clarification in the readme that makes this issue invalid. That pool owners should behave rationally:

Essentially we expect all permissioned actors to behave rationally.

Creating a pool with 128 tokens is certainly not a rational decision.

This is highly irrational from a gas and operational standpoint, and pool creators are expected to understand and act in their best interest when configuring pools.

Planning to accept the escalation and invalidate the issue.

@Joshuajee
Copy link

Joshuajee commented Oct 13, 2024

@cvetanovv

I will start by correcting an error in an earlier calculation made.
It costs $22.8, and not $2.28 to liquidate the position as that's the cost of 1M gas in Ethereum https://www.cryptoneur.xyz/en/gas-fees-calculator?usedGas=20203216&txnType=Custom&gasPrice=standard so the total gas cost to liquidate such position will be $1470, this is a huge sum of money.

I agree that permissioned actors like pool admin will do what is in their best interest but for them to do what is in their best interest they have to know what is in their best interest. They don't know that too much collateral will hurt the pool. In reality, such pools will be very attractive because they give borrowers a wide variety of assets to borrow from by providing a single collateral, and also increase earnings for Lenders as they will earn more because of high borrowing activities in such pools.

Gas cost for pool creation is a one-time fee that will only be paid once so they can forgo that. For example, it costs 20M gas to create this pool which is about 500 USD. But this is a one-time fee.

Let's say the admin sets the pool to use 64 assets i.e half the maximum assets allowed. It will cost them about 10M gas to create the pool which is $250USD, which is a reasonable fee.

Liquidating a single asset will cost 556102 gas which is $13, this is way above the collateral provided by the user, and liquidating the whole position will cost 64 * 13 / 2 = $416.

When such losses occur the pool admin doesn't lose but rather the Liquidity Providers lose. According to the readme if the action of one party can affect the other they want to consider it. Here the action of the Pool admin can affect the LPs and any Vault that integrates such a pool. Even though the bad guy here is the malicious borrower.

There are two sets of actors. Actors who manage pools and actors who mange vaults. If an action done by one party causes the other party to suffer losses we'd want to consider that.

Let's say the admin sets the pool to use 32 assets i.e. 1/4 the maximum assets allowed. It will cost them about 5M gas to create the pool which is $125, which is a reasonable fee.

Liquidating a single asset will cost 331941 gas which is $8, this is way above the collateral provided by the user, and liquidating the whole position will cost 32 * 8 / 2 = $128.

Even at 32 assets which is very likely to occur in real life the liquidation incentive is not there for the liquidators and the user can just make smaller positions multiple times, to exploit such pools.

This exploit is more dangerous at the extremes but it can be done even with 32 assets, whatever the case maybe the protocol needs to prevent this from happening, and I gave two recommended fixes.

The user can choose to provide collateral of less than $100 and attack pools with about 32 assets, and they can repeat this 10 times with 10 different positions.

@cvetanovv
Copy link

@Joshuajee

When I judge an issue, I look at what is written there. There, you wrote a pool with 128 assets.

The readme states that the pool manager is expected to be rational. Certainly, having 128 assets in the pool is not rational.

Even assuming your last comment with a 32-asset pool, how common is it for a pool to have 32 assets?
Would it be a rational decision for pool owners to create pools with so many assets?

@Joshuajee
Copy link

Joshuajee commented Oct 14, 2024

@cvetanovv ,
My bad not putting the other cases in the report.
One of my recommendations was to reduce the total number of collaterals used in a single position to 10. I believed that would be a sufficient mitigation, as for 32 assets pool I think it will be very common when zerolend launches. The only barrier of doing this is the gas cost that only exists on ethereum and $125 is not too much for pool creation. Pools like this have no issues in other chains, and will be very common there, I stated that the rationality behind many asset pools as it gives users a wide range of assets to borrow from with a single collateral, pools with many assets will also out earn pool with fewer assets because of high borrowing activities. Zerolend allows up to 128 assets and they don't plan to limit that number in any way, nor did they advise against it.

@Joshuajee
Copy link

@cvetanovv

I believe that a comment under this issue #233 has established that pool deployers are not Permissioned nor are they expected to behave rationally.

While I still hold my stand that there are reasons for pools to have many assets as high as 32 even 128, for ease of borrowing, knowing fully well that Zerolend plans to support meme coins. I feel the argument about pool deployers being rational no longer stands in light of this new discovery.

@cvetanovv
Copy link

@Joshuajee

The main impact of this issue is that there will be no initiative to liquidate the assets. But in the readme, we can see that the protocol will use bots for this purpose:

Are there any off-chain mechanisms or off-chain procedures for the protocol (keeper bots, arbitrage bots, etc.)?

  • There are liquidation bots that run off-chain to execute liquidations similar to how liquidations work on Aave.

I think your issue is valid, but it is Low severity. A pool with many assets is not a rational behavior, and even if someone decides to make such a pool, there are bots that will be able to liquidate the assets when necessary.

We don't even have a DoS situation here. At worst, the protocol will pay a little more gas. You can see in the Sherlock documentation that this would be an invalid issue:

The user/protocol ends up paying a little extra gas because of this issue.

My decision to accept the escalation and invalidate the issue remains.

@Joshuajee
Copy link

@cvetanovv this is not a little extra gas fee, paying more than the debt to liquidate a position is a huge lose, liquidators are bots and according to AAVE they are advised to calculate the profit from liquidations before going ahead. That being in mind if a user can deliberately discourage liquidators that's and issue. There is no guarantee that zerolend will run all the liquidation bots.

@Joshuajee
Copy link

Joshuajee commented Oct 16, 2024

@cvetanovv
There is a breakdown of feature here as the only incentive to liquidate is the fee. Liquidators are independent bots zerolend is not the liquidator. https://docs.aave.com/faq/liquidations
This is not gas grieving.

@Honour-d-dev
Copy link

@cvetanovv
There is a breakdown of feature here as the only incentive to liquidate is the fee. Liquidators are independent bots zerolend is not the liquidator. https://docs.aave.com/faq/liquidations
This is not gas grieving.

This is aave docs

@cvetanovv
Copy link

@Joshuajee

The protocol will use off-chain bots: https://github.com/sherlock-audit/2024-06-new-scope?tab=readme-ov-file#q-are-there-any-off-chain-mechanisms-or-off-chain-procedures-for-the-protocol-keeper-bots-arbitrage-bots-etc

There is no guarantee that zerolend will run all the liquidation bots. - on the contrary, it is guaranteed that they will use off-chain bots. It is written in the readme.

There are liquidation bots that run off-chain to execute liquidations similar to how liquidations work on Aave.

My decision to accept the escalation remains.

@Joshuajee
Copy link

Joshuajee commented Oct 16, 2024

@cvetanovv
Similar to how liquidations works on AAVE, this is how liquidations work on AAVE.
Screenshot_20241016-112217
This is on the last session of the faq
https://docs.aave.com/faq/liquidations

@cvetanovv
Copy link

@Joshuajee

The readme states similar, not the same as AAVE. They can configure the bots however they want.

These links where you give are not relevant because they are from a different protocol. I'm showing you what they wrote in the Readme, which has the most weight in making a decision.

My decision to accept the escalation remains.

@Joshuajee
Copy link

Joshuajee commented Oct 16, 2024

@cvetanovv
The protocol is a fork of AAVE, and many issues have been disregarded because of that point, the protocol said.

There are liquidation bots that run off-chain to execute liquidations similar to how liquidations work on Aave.

I think that has already told you that they are working with the same model as AAVE.

  • Are there liquidation bots that run off-chain on AAVE? Yes.
  • Are they all owned by AAVE? No

Zerolend never said that they are running the Liquidations themselves.

If I may ask how will zero lend handle liquidation similar to how AAVE works? @cvetanovv, if they don't rely on third-party liquidators like AAVE? Liquidation is permissionless.

@Joshuajee
Copy link

@cvetanovv
Liquidation works on AAVE by incentivizing independent users to create bots and liquidate positions for profit that's how AAVE works and that's what they mean by similar.

@Joshuajee
Copy link

Joshuajee commented Oct 17, 2024

@cvetanovv,

We have been arguing the rationality behind having many asset pools, as high as 128 or 32. Currently, AAVE's largest market, Ethereum Mainnet, has 33 assets. Seven of them are in isolation mode, and four cannot be used as collateral, so there will be 21 active assets that can be used as collateral.

Zerolend is permissionless and is designed to handle more tokens because their documentation talks about handling meme coins, this means that Zerolend should ideally support more.

As for this argument, the liquidators are not paying slightly more gas, they are paying more than the funds that they intend to liquidate, and that's not slightly more, it is a lot more.

The user/protocol ends up paying a little extra gas because of this issue.

Zerolend pools also have the upgradability feature, this means that some pools can be upgraded overtime with more assets being added to it.

@cvetanovv
Copy link

@Joshuajee

This issue has three impacts, which you wrote:

  1. The position will not be liquidated on time.
  2. Many positions will go insolvent without liquidators to close them.
  3. Liquidity providers will lose money.

1 and 2 are invalid because we have bots that will take care of this.

3 is probably the only valid one, but is it enough to be Medium severity? If we have a pool with 30 assets, how much funds would the bot lose in liquidation?

@Joshuajee
Copy link

Joshuajee commented Oct 17, 2024

@cvetanovv

For 30 assets it will cost 317,939 gas for first liquidation.

Given that the price of Ethereum has since increased, it will cost $18.33 for 317939 gas at the time of writing this comment.

The total cost to Liquidate everything would be 30 * 317,939 / 2 = 4769085 i.e. $287.28.

If each asset is worth $10, and the liquidation bonus averages 10%, the liquidation fee will be $1.

The bot will have to pay $9 to receive $10 worth of collateral, $1 serving as their incentive to liquidate the position in the first place.

The gas fee to Liquidate the first position is $18.33, so the bot will be paying $18.33 + $9 = $27.33, given that they get $10 from the liquidation operation. Their loss will be $27.33 - $10 = $17.33, this is the loss from just one collateral, liquidating all 30 collateral will cost $17.33 * 30 / 2 = $252.95.

So the loss is $252.95 using today's price.

I would like to refer back to my earlier comment that used 32 assets and the cost in dollars was $8.33.

The gas fee to Liquidate the first position is $8.33, so the bot will be paying $8.33 + $9 = $17.33, given that they get $10 from the liquidation operation. Their loss will be $17.33 - $10 = $7.33, this is the loss from just one collateral, taking the total 30 collateral will cost $7.33 * 30 / 2 = $109.95.

@DemoreXTess
Copy link

@Joshuajee @cvetanovv This comment shows us that 10% liquidation bonus is not enough for the this kind of a pool. Those parameters can be configured rationally in this kind of pools ( 30 assets in 1 ). For instance 50% LTV, 30% liquidation bonus and 15% liquidation threshold can easily cover the gas cost and return yield for the liquidator. Of course there should be some consequences for the users who use pools which can have 30 assets as liquidity token. These kinds of pools should have lower LTV, higher bonus and lower liquidation threshold and nothing prevent us to choose those variables while pool creation.

@Joshuajee
Copy link

@DemoreXTess, LTV is and liquidation bonus are set per asset and not fixed for the whole pool, generally the more trustworthy an asset is the higher the LTV and the lesser the liquidation bonus.

@cvetanovv
Copy link

There are too many things in this issue, any one of which would make this a low severity individually.

  1. The pool manager must act rationally according to the readme. If it doesn't act rationally, we don't consider it a problem.

  2. We have liquidation bots that will take care of the liquidation process

  3. The attack path is also wrong - Because the gas cost in liquidating the position is far greater than the liquidation bonus, they will not liquidate the position. - the positions will be liquidated by bots.

  4. And most importantly, the impact of the issue is invalid. None of the impacts listed in the issue are correct.

Impact

  • The position will not be liquidated on time.
  • Many positions will go insolvent without liquidators to close them.
  • Liquidity providers will lose money.
  • The position will not be liquidated on time. - This is not true. There are bots that will take care of this
  • Many positions will go insolvent without liquidators to close them. - On the contrary, the bots would make sure to close the position
  • Liquidity providers will lose money. - This is also not true because the bots will take care of the liquidation.

My decision is to accept the escalation and invalidate the issue.

@Joshuajee
Copy link

@cvetanovv ,

We keep going in circles, Zerolend uses AAVE's model for liquidation that gives incentives to Liquidators for closing positions, when the cost of liquidation far exceeds the gain. No bot is going to Liquidate such a position. It is there on the Readme, but you are coming up with arguments to Waterdown that statement.

Bots are designed to profit from Liquidation, when bots see no profit they will abstain from liquidating such positions this will give rise to the following impacts.

Impact
The position will not be liquidated on time.
Many positions will go insolvent without liquidators to close them.
Liquidity providers will lose money.

I have repeatedly stated that it is completely rational to have such pools, because of the high borrowing activities that such pools will have, I used AAVE as an example they have about 33 assets in their pool with about 11 in isolation mode, so 21 active markets on Mainnet.

I further explained that Zerolend pools are upgradable and as time goes on more assets may be added.

@Joshuajee
Copy link

@cvetanovv

If you are a Liquidation Bot driven by profits, will you liquidate a position that will make you trade $10 for $1 profit? Will that make you a rational Liquidator?

@cvetanovv
Copy link

@Joshuajee So, for you, is it rational for a user to create a pool with 30+ assets?

Rational Behavior:
https://www.investopedia.com/terms/r/rational-behavior.asp

Rational Behavior is a decision-making process that is based on making choices that result in the optimal level of benefit or utility for an individual.

If it is rational for pool managers to create a pool with 30+, then I don't know what is not rational behavior.

@Joshuajee
Copy link

Joshuajee commented Oct 24, 2024

@cvetanovv, is AAVE irrational for Having 33 asset pools on Mainnet, will having only 21 active?

Aside from the Risk in this report and the initial cost of setting up such pools, can you please tell me any risk from having many assets in a pool?

I have mentioned the benefits multiple times, more borrowing and lending will happen in large asset pools, as long as all the tokens are trusted.

Zerolend plans to support Meme Coins do you think that that is rational?

@Honour-d-dev
Copy link

Honour-d-dev commented Oct 24, 2024

@Joshuajee

The fact that the protocol themselves will have bots means that liquidations don't always have to be profitable, even if external liquidators won't be willing to take up the liquidation the protocol bots can.

Aside from the Risk in this report and the initial cost of setting up such pools, can you please tell me any risk from having many assets in a pool?

It is also irrational for someone to borrow in 30+ token , you forget/missed that the calculateUserAccountData function is also called on each borrow, making borrowing also gas intensive and withdrawals after repayment gas intensive too as same function will be called on every withdrawal

@Joshuajee
Copy link

Joshuajee commented Oct 24, 2024

@Honour-d-dev, the protocol said

There are liquidation bots that run off-chain to execute liquidations similar to how liquidations work on Aave.

Can you tell me how Liquidation bots on AAVE work?

Nobody is borrowing 30+ assets here, they are supplying 30+ assets to back a single loan.

@DemoreXTess
Copy link

@Joshuajee

The problem is nobody guarantee a profit when a liquidation occurs in the protocol which is a normal behaviour. Another contest can't be used as argument I know but I want to give an example in Cantina Panoptic contest, they already stated that liquidations can be non-profitable in some situations. It's known issue in all the lending protocols and they never guarantee a profit after liquidation.

Secondly, as I stated above it's totally depends on the configurations of liquidation bonus and liquidation threshold. With correct configuration it won't be a problem in this kind of pools.

@Joshuajee
Copy link

@DemoreXTess,

This issue involves a user deliberating making their position unprofitable to liquidate, this didn't just arrive because of gas prices going up, if someone can deliberately do this then it is an issue, and it can be repeated.

Except you want to give a Liquidation Bonus of 100%, I don't see how it will offset the loss that the Liquidator is currently taking.

@cvetanovv
Copy link

My last decision remains, and I do not want to repeat myself again: #231 (comment)

The whole issue is that there is no one to liquidate given positions in a pool with 30+ assets. This is not true, and as explained in the readme, there will be bots that liquidate them.

My decision is to accept the escalation and invalidate the issue.

@Joshuajee
Copy link

Will wait for HoJ to take a final call

@WangSecurity
Copy link

I agree with @cvetanovv that this issue should be low severity.

  1. This comment explains well how this issue can be prevented by the pool admin. Since there are no limitations on admin-set variables, we can expect them to use such values here (I know anyone can create pools, but it doesn't change the fact we should expect them to use appropriate values).
  2. I agree that a pool of even 30 assets is not a rational behaviour. Not even from the standpoint of gas fees but from the operational perspective. It would be very hard to maintain a pool of 30 assets, especially based on the fact that half of these assets can go to 0 at once (i.e. just have a sharp price decrease), and it would be almost impossible to react in these cases.
  3. Also, as stated above, the impacts in the report are that positions will not be liquidated, they will go insolvent, and LPs will lose funds due to this, but this is assumed to be prevented by the liquidators, and we have no idea how they would work and if they take gas costs into account (just interesting fact, there were situations on previous contests where the main argument was that the function X would not be called because the gas for calling it is >50% of the rewards you get for calling it, but it turned out that gas costs will not be taken into account when calling function X, which may be the fact here).

Agree with @cvetanovv's decision here #231 (comment)

@Joshuajee
Copy link

Joshuajee commented Oct 26, 2024

@WangSecurity,

I believe it won't be the fact here and Gas cost will be an important factor in Liquidating a position, the reason for this is that zerolend uses similar liquidation Model as AAVE. In AAVE thirdparty bots compete to liquidate positions, in AAVE's documentation they warn bots to be aware of the gas cost when Liquidating positions .

On reacting to price change, there is the functionality to freeze reserve.

And the above config put forward by #231 (comment), wouldn't still offset the gas cost.

@WangSecurity
Copy link

I believe it won't be the fact here and Gas cost will be an important factor in Liquidating a position, the reason for this is that zerolend uses similar liquidation Model as AAVE. In AAVE thirdparty bots compete to liquidate positions, in AAVE's documentation they warn bots to be aware of the gas cost when Liquidating positions

Yeah, I understand, but it's still a different protocol and how the bots will act around Zerolend is a different question.

My decision remains as above that this is a low-severity issue: it's griefing with the attacker losing their initial capital (cause for their position to go insolvent they will have to suffer a loss), arguably irrational behaviour by the pool admin, speculates on how the bots would liquidate in this situation.

@Joshuajee
Copy link

Are there any off-chain mechanisms or off-chain procedures for the protocol (keeper bots, arbitrage bots, etc.)?
There are liquidation bots that run off-chain to execute liquidations similar to how liquidations work on Aave.

I believe the above statement clearly shows how liquidations will work here. The protocol is a Fork of AAVE and they plan to use a similar liquidation model as AAVE.

This attack is simply to discourage liquidation bots from attempting to liquidate the distressed position. If the bot decides to liquidate the position, it will suffer a huge loss. Bots would rather avoid such losses and go elsewhere.

As for pool admin this attack is still unknown to them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Escalated This issue contains a pending escalation Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

9 participants