search

sherlock-audit / 2023-01-ajna-judging

1 stars 0 forks source link

Jeiwan - Unscaled value of collateral causes invalid LP price when taking and removing liquidity #150

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Jeiwan

medium

Unscaled value of collateral causes invalid LP price when taking and removing liquidity

Summary

When rewarding a taker or calculating the amount of deposit to withdraw and LP tokens to redeem, bucket collateral value is reduced due to unscaling. As a result: the price of an LP token will be reduced; taker will be rewarded with more LP tokens; removing quote tokens will result in a wrong amount of LP tokens burned and a wrong amount of quote tokens redeemed.

Vulnerability Detail

The pools are designed to hold two types of collateral:

  1. Collateral that's added via the addCollateral function and that's used in trading.
  2. Collateral that's added via the drawDebt function and that's used to take loans.

The collateral of the first type is also counted in LP price calculation (Buckets.sol#L150): it's priced in terms of the quote token, and the value is added to the total amount of quote token deposits of the bucket.

The amount of total bucket deposits can be scaled and unscaled: since quote token deposits accrue borrower interest, the total price of deposits increases with time. Scaled deposits include the accrued interest, unscaled deposits don't. The amount of the collateral of the first type, while contributing to the price of an LP token, doesn't scale because it doesn't accrue interest. However, there are cases when collateral value is downscaled:

  1. when rewarding a bucket take;
  2. when removing deposited quote tokens.

In both of these situations, an LP token exchange rate will be calculated incorrectly due to downscaling of collateral value: one LP token will cost less quote tokens than expected.

Impact

  1. LP tokens amount rewarded to a taker will be greater than it should be (Auctions.sol#L1335).
  2. LP tokens amount redeemed when removing deposits will be greater than it should be (LenderActions.sol#L651).

    Code Snippet

    Buckets.sol#L162-L173 Auctions.sol#L1335 LenderActions.sol#L651

    Tool used

    Manual Review

    Recommendation

    Consider using the Buckets.getExchangeRate function to calculated exchange rate with unscaled bucket deposits.

grandizzy commented 1 year ago

Deposits are stored as unscaled values in the Fenwick tree, but are used to track values including the scale factor. Either scaled or unscaled values can be used as long as the appropriate factor are included in the exchange rates to translate to LPs. In some cases, it's more convenient to work with unscaled values for rounding purposes. The code appears to correctly use the scaled/unscaled deposits and exchange rates where cited.

hrishibhat commented 1 year ago

Closing based on Sponsor comment