cergyk - LibTWAPOracle::setPool Dos on setPool by donating a few wei of 3CRV directly to the metapool

[sherlock-admin2]

cergyk

medium

LibTWAPOracle::setPool Dos on setPool by donating a few wei of 3CRV directly to the metapool

Summary

LibTWAPOracle::setPool relies on a perfect match between the initial balance of uAD and 3CRV in the pool, which means that a malicious user can front-run the admin call to setPool and donate a few wei of 3CRV to the pool in order to make the call revert.

Vulnerability Detail

We can see the the admin function LibTWAPOracle::setPool, checks that the balances of uAD and 3CRV are perfectly equal:
https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibTWAPOracle.sol#L51

This means that a malicious user can front-run the call to setPool by donating a few wei of 3CRV directly to the metapool, in which case the balances will not match. The admin will not be able to set the pool as the function will revert

Impact

The feature of migrating to a new pool will be DoSed temporarily, as long as the attacker keeps sending a few wei to the metapool contract

Code Snippet

Tool used

Manual Review

Recommendation

Accept a slight imbalance between the balances of uAD and 3CRV (by a few dollars), in order to make this kind of DoS economically impractical for the attacker

[sherlock-admin2]

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

auditsea commented:

The issue describes about DOSing setPool function by manipulating the Curve pool, but it's assumed that the Curve pool deployment, LP deposit, and setPool will be handled in one tx using multicall structure

[sherlock-admin2]

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

auditsea commented:

The issue describes about DOSing setPool function by manipulating the Curve pool, but it's assumed that the Curve pool deployment, LP deposit, and setPool will be handled in one tx using multicall structure

[rndquu]

@gitcoindev @molecula451

The recommendation states to "Accept a slight imbalance between the balances of uAD and 3CRV" but this can also lead to potential DOS.

I think we can simply remove this line.

[gitcoindev]

@gitcoindev @molecula451

The recommendation states to "Accept a slight imbalance between the balances of uAD and 3CRV" but this can also lead to potential DOS.

I think we can simply remove this line.

I did a research on GitHub and indeed other pool implementations just use

require(_reserve0 > 0 && _reserve1 > 0, "No reserves");

also some pools check for overflow

therefore since 2 lines above this line we check for reserves it is safe to remove this line.

[rndquu]

@gitcoindev @molecula451
The recommendation states to "Accept a slight imbalance between the balances of uAD and 3CRV" but this can also lead to potential DOS.
I think we can simply remove this line.

I did a research on GitHub and indeed other pool implementations just use

require(_reserve0 > 0 && _reserve1 > 0, "No reserves");

also some pools check for overflow

therefore since 2 lines above this line we check for reserves it is safe to remove this line.

This overflow check is needed because of downcasting from uint256 to uint128 here. I guess this is not the case for us since we don't use uint128 in the LibUbiquityPool.

[nevillehuang]

• If flashbots are considered like @Czar102 previously mentioned, I agree that this issue can be low severity
• If not, this can possibly allow permanent DoS as long as user keep front-running on mainnet and donating funds into the pool, so can possibly be medium severity too

[Czar102]

I agree with the escalation. This issue has the same impact as front-running initializers, which is making the deployment useless (easily detectable) without a loss of funds. This impact is not accepted by Sherlock rules as a Medium severity issue, hence planning to consider this a low.
Will modify the guidelines to include a more generalized rule.

Planning to accept the escalation and invalidate the issue.

[Czar102]

Result:
Low
Has duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• jingyi2811: accepted

[ubiquibot]

@molecula451 the deadline is at 2024-02-17T13:22:13.234Z

[ubiquibot]

@rndquu the deadline is at 2024-02-17T13:22:20.571Z

[ubiquibot]

# No linked pull requests to close

[ubiquibot]

+ Evaluating results. Please wait...

[ubiquibot]

[ 1.1 WXDAI ]

@molecula451

Contributions Overview

View	Contribution	Count	Reward
Issue	Comment	1	1.1

Conversation Incentives

Comment	Formatting	Relevance	Reward
PR Confirmation fix: https://github.com/ubiquity/ubiquity-dollar...	1.1	0.65	1.1

[ 8.4 WXDAI ]

@gitcoindev

Contributions Overview

View	Contribution	Count	Reward
Issue	Comment	1	8.4

Conversation Incentives

Comment	Formatting	Relevance	Reward
> @gitcoindev @molecula451 > > The recommendation states to "...	8.4 a: count: 3 score: "3" words: 7 code: count: 1 score: "1" words: 13	0.695	8.4

[ 42.4 WXDAI ]

@rndquu

Contributions Overview

View	Contribution	Count	Reward
Issue	Task	1.00	25
Issue	Comment	2	0
Issue	Comment	2	17.4

Conversation Incentives

Comment	Formatting	Relevance	Reward
@gitcoindev @molecula451 The recommendation states to "Accep...	- a: count: 1 score: "0" words: 1	0.86	-
> > @gitcoindev @molecula451 > > The recommendation states to "...	- a: count: 5 score: "0" words: 9 code: count: 5 score: "0" words: 17	0.81	-
@gitcoindev @molecula451 The recommendation states to "Accep...	4.3 a: count: 1 score: "1" words: 1	0.86	4.3
> > @gitcoindev @molecula451 > > The recommendation states to "...	13.1 a: count: 5 score: "5" words: 9 code: count: 5 score: "5" words: 17	0.81	13.1

[sherlock-admin]

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

[sherlock-admin]

The Lead Senior Watson signed off on the fix.