cergyk - UbiquityPool::redeemDollar DoS on redeeming if USDT/USDC depegs

[sherlock-admin]

cergyk

medium

UbiquityPool::redeemDollar DoS on redeeming if USDT/USDC depegs

Summary

uAD is algorithmically dependent on its accepted collateral, which are supposed to be LUSD and DAI at first. However there is a mechanism blocking withdrawal in case the TWAP price of uAD in the metapool uAD/3CRV is too high. Since 3CRV also contains USDT and USDC, in case any of USDC or USDT depegging, 3CRV price against uAD will be lower. Redeeming uAD may end up temporarily locked.

Vulnerability Detail

We can see that calling redeemDollar reverts if the price of uAD is above a given threshold:
https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L418-L420

This means that an event which causes the price of 3CRV to be below the threshold such as USDT or USDC depegging will cause redeeming to revert effectiely causing a DOS on withdrawals.

Impact

Temporary DOS on withdrawal because of a depeg of USDT/USDC which should be unrelated.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L418-L420

Tool used

Manual Review

Recommendation

Remove the reliance on 3CRV, create a tricrypto pool which contains collateral tokens

[sherlock-admin2]

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

auditsea commented:

The issue describes about the protocol insolvancy in case of collateral depeg. It's not avoidable, that's why the protocol has borrowing function to get yield, take fees on mint and redeem, these features will hedge the risk from protocol insolvancy

[sherlock-admin2]

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

auditsea commented:

The issue describes about the protocol insolvancy in case of collateral depeg. It's not avoidable, that's why the protocol has borrowing function to get yield, take fees on mint and redeem, these features will hedge the risk from protocol insolvancy

[nevillehuang]

See also #61 for a coded PoC, both talking about UAD price reliant on 3CRV

[rndquu]

Ok, we've already discussed it here so the issue seems to be valid and "will fix".

Not sure about the possible solution, but the following seem worth to check:

• 0xpiken - Incorrect value was used to represent USD price of Ubiquity Dollar #30
• KingNFT - The formula used in getDollarPriceUsd() is incorrect #61
• evmboi32 - Dollar token won't be pegged to 1 USD. #141
• Arz - The TWAP oracle uses 3CRV which is not exactly pegged to $1.00 #165
• fugazzi - TWAP oracle returns incorrect price #192

[0x3agle]

Issue #221 is incorrectly set as a duplicate of this one.

• That issue is concerned about uAD never being pegged to $1 (also accepted by the devs as an issue) and is certain to happen.
• This issue presents a rare (but possible) scenario of a blue-chip stablecoin depeg.

This issue is invalid:

• The UbiquityPool price of uAD does not depend on the "price" of 3CRV, instead, it depends on the reserve balances of 3CRV and uAD.
• If the price of 3CRV drops, the pool is still balanced as the quantity is not changed. Therefore de-pegging USDT/USDC will not DoS the redeems.
• The TWAPOracle calculates uAD price (ts.price0Average) based on the balances and not on the actual "prices"
  
• The redeems will be DoS'ed if there is a meager amount of uAD in the curve pool. This can be corrected by depositing uAD (which can be minted from Ubiquity Pool as the uAD price has increased).

[osmanozdemir1]

Escalate

1. I wanted to use my escalation for the comment above.
2. Even if this issue is valid and the above comment is wrong, this issue should be a medium not high since it requires a major external factor and the Dos is temporary.

[sherlock-admin2]

Escalate

1. I wanted to use my escalation for the comment above.
2. Even if this issue is valid and the above comment is wrong, this issue should be a medium not high since it requires a major external factor and the Dos is temporary.

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.

[0xArz]

Escalate

1. I wanted to use my escalation for the comment above.
2. Even if this issue is valid and the above comment is wrong, this issue should be a medium not high since it requires a major external factor and the Dos is temporary.

What is the major external factor if 3CRV is already depegged and its not exactly $1.00 but $1.03? Correct me if im wrong but when the pool is created and the balances are equal the price of uAD will be $1.03 because the price of 3CRV is $1.03, this will cause the pool to be rebalanced so that for 1 uAD you get 0.97 3CRV and the price of uAD is actually $1.00 not $1.03. The twap will then return 0.97 and minting will be DoSed

[osmanozdemir1]

External factor is something else being depegged. “already” doesn’t mean anything.

There is no issue without an external problem.

[0x3agle]

@0xArz You are correct.

The uAD is backed by a token with value of $1.03.
And the UbiquityPool considers that uAD is $1.

This discrepancy is really not ideal for a stable-coin which is what was pointed out in #221

[0xLogos]

It wouldn't. TWAP + thresholds only determine when user can mint/redeem uAD.

When pool twap > mint threshold uAD above peg and users can mint X uAD providing X$ equivalent according to chainlink price amount of collateral. Since uAD above beg X uAD > X$ and users take profit by selling uAD on external markets and eventually lowering twap.

So once redeem/mint open peg target is 1$ according to chailink oracle.

[ydspa]

@0xLogos but the uAD denomination would change from my understanding – 1 uAD would be e.g. $1.05 instead of $1. There is no direct loss of funds, but core functionality is impacted.

Correct, actually the contract would be bricked at the current mainnet environment, as the mint() would always revert due to incorrect return of getDollarPriceUsd().

Furthermore, the front end and users would rely on getDollarPriceUsd() to get uAD/USD price (we can expect there is no chainlink uAD/USD feed in short term), and users might be misled to suffer loss. e.g. if the true uAD/USD is $1.05 but reported $1, users should sell on market instead of redeem().

[ydspa]

@Czar102 I believe all are valid issues, even #192, which correctly identifies the root cause, albeit not explicitly mentioning 3CRV

A vague touch of the issue with low quality reports should be invalid dups. e.g. sherlock-audit/2023-09-Gitcoin-judging#458 there are hundreds submissions identifying the root causes but only some of them are valid.

[Czar102]

@0xLogos thank you for this argument. Since the issue can be easily remedied by changing the thresholds and doesn't cause any loss of funds, I think it's fair to consider it a low severity issue.

[nevillehuang]

@Czar102 Why would it be a good idea/acceptable to allow adjustment of price thresholds further away from the intended narrow range of 0.99-1.01? TWAP is the only representation of uAD price, isn’t that extremely important to the protocol?

[Czar102]

TWAP is the only representation of uAD price, isn’t that extremely important to the protocol?

That has been the discussion, and from my understanding TWAP is only used for a sanity check of mint/redeem thresholds.
And increasing thresholds would just counter the 3CRV token price increase to make the thresholds denominated in uAD 0.99-1.01.

That's my current understanding of this issue.

[nevillehuang]

@Czar102 @osmanozdemir1 @CergyK

Since uAD are always minted 1:1 to collateral value, and TWAP is the only representation of price with secondary markets being expected based on sponsors comments for arbitrage purposes, consider this scenario:

1. Sell uAD at $1.03 (since it is pegged to 3CRV at current prices of $1.03) on secondary markets
2. Buy more collateral
3. Mint uAD with inflated collateral. Profit because of inaccurate prices (when they are not supposed to, given price should be reflected as $1) and unnecessarily inflate uAD supply.
4. Repeat steps 1 to 3, since uAD price will never be accurately reflected given its dependency on 3CRV

It is crucial for threshold checks to be within protocols desired price ranges to reflect and keep accurate uAD prices

[sherlock-admin]

The protocol team fixed this issue in PR/commit ubiquity/ubiquity-dollar#893.

[0xArz]

3CRV is quite volatile and the wrong thresholds will prevent arbitrageurs from stablizing the stablecoin which is the whole point of this UbiquityPool.

The curve pool will be launched with 50:50 reserves(there is a check in the twap), this means that the price of uAD will be `$1.03. Because uAD is supposed to be a stablecoin the curve pool should get rebalanced so that for 1 3CRV you get 0.97 uAD. To do this you would need to sell uAD although no one will not be able to mint uAD from the UbiquityPool because of the thresholds and the TWAP returns 1 because the reserves are balanced. The admin would then have to adjust the thresholds - 0.96 for redeeming and 0.98 for minting. If the twap returns 0.97 then the reserves are balances so that the value of both reserves is 1$.

Now lets say if the price of 3CRV goes to $1.04, the curve pool will need to be rebalanced again by minting uAD which will not be possible because of the thresholds which need to be adjusted again.

If the price of 3CRV goes to $1.02 then the abitrage bot will need to buy uAD and then redeem it for collateral but again because of the wrong threshold the arbitrage bot will not be able to redeem.

[ydspa]

TWAP is the only representation of uAD price, isn’t that extremely important to the protocol?

That has been the discussion, and from my understanding TWAP is only used for a sanity check of mint/redeem thresholds. And increasing thresholds would just counter the 3CRV token price increase to make the thresholds denominated in uAD 0.99-1.01.

That's my current understanding of this issue.

It's absolutely not just a simple sanity check, but works as an important protection for uAD stability. As mentioned by @nevillehuang, since uAD are always minted/redeemed 1:1 to collateral ($LUSD). Let's say, at initial market the LUSD = uAD = $1, then if price of LUSD drift to $0.95, then users can mint lots of uAD and sell on the uAD/3CRV pool to get profit, and finally make price uAD drift to $0.95 too. But while the protection works, no more uAD could be minted while uAD price drifts to $0.99. Therefore, we can't simply adjust the mint/redeem threshold to something like 0.96 to solve this problem, doing this would make the security design barely works, as my view, the current 0.99/1.01 threshold is almost the max acceptable config for a stable coin, so a correct getDollarPriceUsd() is important.

[0xLogos]

from my understanding TWAP is only used for a sanity check of mint/redeem thresholds. And increasing thresholds would just counter the 3CRV token price increase to make the thresholds denominated in uAD 0.99-1.01.

Yeap, that's exactly what I wanted to say

3CRV is quite volatile...

First of all, it's different issue. Second I believe it's more suboptimal choice rather than vulnerability bcz you can't say (take into account all market forces) for sure how it will go until deployed. If it's inefficient, nothing bad will happen, just new solution has to be found.

TWAP is the only representation of uAD price

Price is what you pay when buy. TWAP is market indicator and used for special purposes. Maybe it was intended to be used in frontend or something else too, but it's out of scope. In contracts it's only used for (sanity) check.

[0x3agle]

I don't understand why the validity is still being argued upon given that the protocol team confirmed the issue and has already created a PR to fix it.

[Czar102]

Let's say, at initial market the LUSD = uAD = $1, then if price of LUSD drift to $0.95, then users can mint lots of uAD and sell on the uAD/3CRV pool to get profit, and finally make price uAD drift to $0.95 too.

@ydspa from my understanding, they would need to pay a little more than 1.05 LUSD for every uAD (Chainlink oracle), so the price of uAD would be maintained.

I'm still convinced by @0xLogos's arguments. The impact of changed TWAP due to the initial offset or accruing fees can be easily countered. Planning to consider this a low severity issue.

[ydspa]

@ydspa from my understanding, they would need to pay a little more than 1.05 LUSD for every uAD (Chainlink oracle), so the price of uAD would be maintained.

Nope, uAD always 1:1 minted by LUSD, this is why the threshold protection existing as LUSD might drift from $1, and the protection strop uAD from drifting together with LUSD.

I'm still convinced by @0xLogos's arguments. The impact of changed TWAP due to the initial offset or accruing fees can be easily countered. Planning to consider this a low severity issue.

how to countered, set the mint threshold to 0.96? and uAD has no chainlink oracle, getDollarPriceUsd() is the only representation of uAD price for users and front end. Just this point, it's a break of core feature.

It is also not just a initial offset or accruing fees problem, the difference with real price is dynamic due to uses' action on curve 3pool, such as unbalance of USDC/USDT/DAI in 3pool

[0xArz]

@Czar102 The initial offset might work but this still doesnt solve the issue that uAD is pegged to 3CRV which is not a stablecoin and is volatile as you can see from the charts. Please read my comment again here. Because of the wrong thresholds arbitrage will not be possible and this will prevent uAD from stabilizing which is the whole point of the UbiquityPool and its crucial. Adjusting the thresholds manually will be too slow and problematic when the volatility is high

[detectiveking123]

I have very little context here, but any issue that relies on an established stable-coin depegging should be a low.

[Czar102]

When the thresholds are wrong, setting them to proper ones doesn't cause any loss of funds to anyone. The price may be temporarily off, but as soon as the thresholds are changed, it should stabilize without loss of funds.

[ubiquibot]

! No price label has been set. Skipping permit generation.

[Czar102]

Result:
Low
Has duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• osmanozdemir1: accepted

[sherlock-admin]

The Lead Senior Watson signed off on the fix.