ajna-finance / ajna-core

The Ajna protocol is a non-custodial, peer-to-peer, permissionless lending, borrowing and trading system that requires no governance or external price feeds to function.
https://www.ajna.finance/
Other
31 stars 11 forks source link

Borrower with threshold price less than `MIN_PRICE` cannot be kicked. #973

Closed prateek105 closed 1 year ago

prateek105 commented 1 year ago

Description

Purpose

In order for the borrower’s position to be kicked, it should be under-collateralized with respect to the current LUP. However, as per the pool constraints, LUP cannot go lower than MIN_PRICE. Thus, positions with threshold price below MIN_PRICE can’t ever be kicked, irrespective of kicker or lender actions. Even when lenderKick is being used, the total debt is increased by the deposit amount and starts to exceed the overall deposit in the pool, LUP will still be bounded by the MIN_PRICE.

Recommendation

Allow to kick any position, irrespective of its collateralization, if the LUP index ever reaches MAX_FENWICK_INDEX.

* Solves what we internally have called Dmitrii - 9, an issue discovered by @dmitriia (although not eligible for the Sherlock report) -> 

Summary of the Vulnerability: The code outlines a scenario where a borrower can exploit the auction mechanism post-default to incur minimal penalties and unfairly benefit at the expense of the lender (kicker). This is achieved by the borrower exiting the auction early at an artificially high price, avoiding significant taker penalties, and causing collateral damage to the lender. The penalty paid by the borrower is negligible compared to the advantage gained, rendering the penalty almost irrelevant.

Link an example of the issue: https://github.com/ajna-finance/contracts/pull/981/files#diff-e59157bfa5bad910c2f5c2a00cfba7d222f0ad6f1969eb5ab4b2fbd22903b951R212

Sudo Code Representation of the Attack:

  1. Lender adds liquidity to the pool.

  2. Borrower draws a large debt from the pool.

  3. Time skip of 100 days.

  4. Lender initiates a kick due to default.

  5. Borrower exploits the situation by depositing into a high price bucket.

  6. Borrower takes from the bucket, reducing their penalty significantly.

  7. Borrower removes a large portion of the collateral at an inflated price.

  8. Post-removal, the lender's position is significantly weakened, with minimal gains.

  9. Borrower ends up with a minor loss in USDC and almost no loss in WETH.

  10. Lender gains are marginal compared to the potential loss incurred.

Analysis:

This vulnerability creates an imbalance in the protocol, favoring borrowers who default and strategically manipulate the auction process, while significantly disadvantaging lenders who face the brunt of the collateral damage. The protocol needs to address this imbalance to ensure fair and secure interactions between all parties involved.



## Impact

<!-- State technical consequences of the change, whether beneficial or detrimental.  For example:
_Small increase in `removeQuoteToken` gas cost._
If the change does not affect deployed contracts, feel free to leave _none_. -->

## Tasks

- [x] Changes to protocol contracts are covered by unit tests executed by CI.
- [x] Protocol contract size limits have not been exceeded.
- [x] Gas consumption for impacted transactions have been compared with the target branch, and nontrivial changes cited in the _Impact_ section above.
- [x] Scope labels have been assigned as appropriate.
- [x] Invariant tests have been manually executed as appropriate for the nature of the change.
prateek105 commented 1 year ago

The point of the change is to ensure loans with a TP below MIN_PRICE can be kicked if they go below the LUP when LUP == MIN_PRICE. Is there an existing unit test which exercises that use case?

I have added a unit test here https://github.com/ajna-finance/contracts/pull/973/commits/1d0120f8bfa4149cc7fa9a6cf8e7a393f8f20c1b @EdNoepel