search

sherlock-audit / 2024-08-velar-artha-judging

9 stars 3 forks source link

4gontuk - LPs will withdraw more value than deposited during pegged token de-peg events #52

Open sherlock-admin2 opened 2 months ago

sherlock-admin2 commented 2 months ago

4gontuk

Medium

LPs will withdraw more value than deposited during pegged token de-peg events

Summary

The CONTEXT function in gl-sherlock/contracts/api.vy uses the <quote-token>/USD price for valuation, assuming a 1:1 peg between the quote token and USD. This assumption can fail during de-peg events, leading to incorrect valuations and potential exploitation.

Root Cause

The CONTEXT function calls the price function from the oracle contract to get the price of the quote token. This price is adjusted based on the quote_decimals, implying it is using the <quote-token>/USD price for valuation.

Detailed Breakdown

  1. CONTEXT Function in api.vy: The CONTEXT function calls the price function from the oracle contract to get the price of the quote token.
def CONTEXT(
    base_token : address,
    quote_token: address,
    desired    : uint256,
    slippage   : uint256,
    payload    : Bytes[224]
) -> Ctx:
  base_decimals : uint256 = convert(ERC20Plus(base_token).decimals(), uint256)
  quote_decimals: uint256 = convert(ERC20Plus(quote_token).decimals(), uint256)
  # this will revert on error
  price         : uint256 = self.ORACLE.price(quote_decimals,
                                              desired,
                                              slippage,
                                              payload)
  return Ctx({
    price         : price,
    base_decimals : base_decimals,
    quote_decimals: quote_decimals,
  })
  1. price Function in oracle.vy: The price function in oracle.vy uses the extract_price function to get the price from the oracle.

########################################################################
TIMESTAMP: public(uint256)

@internal
def extract_price(
    quote_decimals: uint256,
    payload       : Bytes[224]
) -> uint256:
  price: uint256 = 0
  ts   : uint256 = 0
  (price, ts) = self.EXTRACTOR.extractPrice(self.FEED_ID, payload)

  # Redstone allows prices ~10 seconds old, discourage replay attacks
  assert ts >= self.TIMESTAMP, "ERR_ORACLE"
  self.TIMESTAMP = ts

  # price is quote per unit base, convert to same precision as quote
  pd   : uint256 = self.DECIMALS
  qd   : uint256 = quote_decimals
  s    : bool    = pd >= qd
  n    : uint256 = pd - qd if s else qd - pd
  m    : uint256 = 10 ** n
  p    : uint256 = price / m if s else price * m
  return p

########################################################################
PRICES: HashMap[uint256, uint256]

@internal
def get_or_set_block_price(current: uint256) -> uint256:
  """
  The first transaction in each block will set the price for that block.
  """
  block_price: uint256 = self.PRICES[block.number]
  if block_price == 0:
    self.PRICES[block.number] = current
    return current
  else:
    return block_price

########################################################################
@internal
@pure
def check_slippage(current: uint256, desired: uint256, slippage: uint256) -> bool:
  if current > desired: return (current - desired) <= slippage
  else                : return (desired - current) <= slippage

@internal
@pure
def check_price(price: uint256) -> bool:
  return price > 0

# eof
  1. extract_price Function in oracle.vy: The extract_price function adjusts the price based on the quote_decimals, which implies it is using the <quote-token>/USD price for valuation.
def extract_price(
    quote_decimals: uint256,
    payload       : Bytes[224]
) -> uint256:
  price: uint256 = 0
  ts   : uint256 = 0
  (price, ts) = self.EXTRACTOR.extractPrice(self.FEED_ID, payload)

  # Redstone allows prices ~10 seconds old, discourage replay attacks
  assert ts >= self.TIMESTAMP, "ERR_ORACLE"
  self.TIMESTAMP = ts

  # price is quote per unit base, convert to same precision as quote
  pd   : uint256 = self.DECIMALS
  qd   : uint256 = quote_decimals
  s    : bool    = pd >= qd
  n    : uint256 = pd - qd if s else qd - pd
  m    : uint256 = 10 ** n
  p    : uint256 = price / m if s else price * m
  return p

Impact

During a de-peg event, LPs can withdraw more value than they deposited, causing significant losses to the protocol.

Attack Path

  1. Deposit: Attacker deposits 1 BTC and 50,000 USDT when 1 BTC = 50,000 USD.
  2. De-peg Event: The pegged token (USDT) de-pegs to 0.70 USD.
  3. Withdraw: Attacker withdraws their funds, exploiting the incorrect assumption that 1 USDT = 1 USD.

Proof of Concept (PoC)

  1. Deposit:
@external
def mint(
  base_token  : address, #ERC20
  quote_token : address, #ERC20
  lp_token    : address, #ERC20Plus
  base_amt    : uint256,
  quote_amt   : uint256,
  desired     : uint256,
  slippage    : uint256,
  payload     : Bytes[224]
) -> uint256:
  """
  @notice            Provide liquidity to the pool
  @param base_token  Token representing the base coin of the pool (e.g. BTC)
  @param quote_token Token representing the quote coin of the pool (e.g. USDT)
  @param lp_token    Token representing shares of the pool's liquidity
  @param base_amt    Number of base tokens to provide
  @param quote_amt   Number of quote tokens to provide
  @param desired     Price to provide liquidity at (unit price using onchain
                     representation for quote_token, e.g. 1.50$ would be
                     1500000 for USDT with 6 decimals)
  @param slippage    Acceptable deviaton of oracle price from desired price
                     (same units as desired e.g. to allow 5 cents of slippage,
                     send 50000).
  @param payload     Signed Redstone oracle payload
  """
  ctx: Ctx = self.CONTEXT(base_token, quote_token, desired, slippage, payload)
  return self.CORE.mint(1, base_token, quote_token, lp_token, base_amt, quote_amt, ctx)

  1. De-peg Event: The pegged token de-pegs to 0.70 USD (external event).

  2. Withdraw:

def burn(
  base_token  : address,
  quote_token : address,
  lp_token    : address,
  lp_amt      : uint256,
  desired     : uint256,
  slippage    : uint256,
  payload     : Bytes[224]
) -> Tokens:
  """
  @notice            Withdraw liquidity from the pool
  @param base_token  Token representing the base coin of the pool (e.g. BTC)
  @param quote_token Token representing the quote coin of the pool (e.g. USDT)
  @param lp_token    Token representing shares of the pool's liquidity
  @param lp_amt      Number of LP tokens to burn
  @param desired     Price to provide liquidity at (unit price using onchain
                     representation for quote_token, e.g. 1.50$ would be
                     1500000 for USDT with 6 decimals)
  @param slippage    Acceptable deviaton of oracle price from desired price
                     (same units as desired e.g. to allow 5 cents of slippage,
                     send 50000).
  @param payload     Signed Redstone oracle payload
  """
  ctx: Ctx = self.CONTEXT(base_token, quote_token, desired, slippage, payload)
  return self.CORE.burn(1, base_token, quote_token, lp_token, lp_amt, ctx)
  1. Incorrect Price Calculation:
def CONTEXT(
    base_token : address,
    quote_token: address,
    desired    : uint256,
    slippage   : uint256,
    payload    : Bytes[224]
) -> Ctx:
  base_decimals : uint256 = convert(ERC20Plus(base_token).decimals(), uint256)
  quote_decimals: uint256 = convert(ERC20Plus(quote_token).decimals(), uint256)
  # this will revert on error
  price         : uint256 = self.ORACLE.price(quote_decimals,
                                              desired,
                                              slippage,
                                              payload)
  return Ctx({
    price         : price,
    base_decimals : base_decimals,
    quote_decimals: quote_decimals,
  })

Mitigation

To mitigate this issue, the protocol should use the <base-token>/<quote-token> price directly if available, or derive it from the <base-token>/USD and <quote-token>/USD prices. This ensures accurate valuations even if the quote token de-pegs from USD.

mePopye commented 2 months ago

Escalate

On behalf of the watson

sherlock-admin3 commented 2 months ago

Escalate

On behalf of the watson

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.

WangSecurity commented 1 month ago

After additionally considering this issue, here's my understanding. Let's assume a scenario of 30% depeg and USDT = 0.7 USD.

  1. The pool has 10 BTC and 500k USDT.
  2. User deposits 1 BTC and 50k USDT, assuming 1 BTC = 50k USDT = 50k USD.
  3. USDT depegs to 0.7 USD, i.e. 1 USDT = 0.7 USD. Then BTC = 50k USD = ~71.5k USDT.
  4. The user withdraws 1 BTC and 50k USDT. They receive it because the code still considers 1 USDT = 1 USD. There are 10 BTC and 500k USDT left in the contracts.
  5. The protocol didn't lose any funds, the amount of BTC and USDT remained the same as it was before the depeg.
  6. But, in reality, the user has withdrawn 50k worth of BTC and 35k worth of USDT since 1 USDT = 0.7 USD.
  7. Hence, if the protocol accounted for the depeg, there had to be 10 BTC and 515k USDT left in the contract after the user had withdrawn during the depeg.

Hence, even though it's not a direct loss of funds but a loss in value, this should be a valid medium (considering depeg as an extensive limitation). Thus, planning to accept the escalation and validate with medium severity. The duplicate is #113, are there any additional duplicates?

WangSecurity commented 1 month ago

Result: Medium Has duplicates

sherlock-admin2 commented 1 month ago

Escalations have been resolved successfully!

Escalation status: