KupiaSec - The protocol should consider the variance of quote tokens' price

[sherlock-admin3]

KupiaSec

Medium

The protocol should consider the variance of quote tokens' price

Summary

The protocol uses the USD price of base token and it assumes quote token is stablecoin: fixed conversion rate of 1 USD = 1 quote token.
But the price of stablecoin can be changed.
There is a recent depeg event in March 2023, where USDC price went as low as 87 cents(Reference).
As a result, if there is variance of quote token, all the conversion between base and quote is incorrect and this can break the protocol's design.

Root Cause

In oracle.vy:89, it returns the USD price of base token regardless of quote token's price.

Internal pre-conditions

None

External pre-conditions

None

Attack Path

None

Impact

The quote token's price variance causes incorrect calculation of token amount and breaks the protocol's design.

PoC

The oracle provides the USD price of base token and the protocol uses the USD price regardless of quote token's price here.

def base_to_quote(tokens: uint256, ctx: Ctx) -> uint256:
  lifted : Tokens  = self.lift(Tokens({base: tokens, quote: ctx.price}), ctx)
  amt0   : uint256 = self.to_amount(lifted.quote, lifted.base, self.one(ctx))
  lowered: Tokens  = self.lower(Tokens({base: 0, quote: amt0}), ctx)
  return lowered.quote
def quote_to_base(tokens: uint256, ctx: Ctx) -> uint256: # tokens(0, 100ke6), ctx(50ke12, 18, 6)
  l1     : Tokens  = self.lift(Tokens({base: 0, quote: tokens}),    ctx) # tokens(0,100ke18) 
  l2     : Tokens  = self.lift(Tokens({base: 0, quote: ctx.price}), ctx) # tokens(0,50ke18) 
  vol0   : uint256 = self.from_amount(l1.quote, l2.quote, self.one(ctx)) # 100ke18 * 1e18 / 50ke18 = 2e18
  lowered: Tokens  = self.lower(Tokens({base: vol0, quote: 0}), ctx) # 2e18
  return lowered.base

If the quote token's price is changed, these functions return incorrect value.

Mitigation

Ideally, there needs to be an additional oracle to check current Price of quote token and take it's price into the consideration.

Duplicate of #52

[KupiaSecAdmin]

Escalate

This issue deserves Medium.
Information required for this issue to be rejected.

[sherlock-admin3]

Escalate

This issue deserves Medium.
Information required for this issue to be rejected.

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]

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 duplicate with #52, since it does a better job of explaining the issue.
Are there additional duplicates?

[WangSecurity]

Result:
Medium
Duplicate of #52

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• KupiaSecAdmin: accepted