Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cvx3CrvOracle returns 0 for small baseAmount #93

Open
code423n4 opened this issue Jan 30, 2022 · 2 comments
Open

Cvx3CrvOracle returns 0 for small baseAmount #93

code423n4 opened this issue Jan 30, 2022 · 2 comments
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Handle

kenzo

Vulnerability details

Due to the precision of Chainlink oracle and Curve's virtual price,
Cvx3CrvOracle will return 0 for very small baseAmount.

Impact

As it only happens with very small amounts, the impact is not big as far as I can tell.
However one can imagine for example a scenario where a user will request to buy a small amount of Cvx3Crv, and a contract would use peek to know how much to charge. Since peek returns 0, the user wouldn't be charged anything and still be able to receive a small amount of Cvx3Crv.

Proof of Concept

For example, for real (taken from contracts) values of:

minStable = 388135165257710
virtualPrice = 1020217504409323943
baseAmount = 1000

The oracle will return 0.

Recommended Mitigation Steps

Consider reverting the transaction if baseAmount > 0 and quoteAmount == 0.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jan 30, 2022
code423n4 added a commit that referenced this issue Jan 30, 2022
@alcueca
Copy link
Collaborator

alcueca commented Feb 2, 2022

That's true, and an issue that no one else saw in any of the previous audits, given that this was first done in the ChainlinkMultiOracle.

While I think there might be little risk about it, we'll think more deeply about it, and I personally appreciate you seeing this.

@alcueca alcueca added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Feb 2, 2022
@GalloDaSballo
Copy link
Collaborator

I believe the finding to be valid, given the limited scope am conflicted on giving medium severity.

In the example given by the warden
uint256 price = (threecrv.get_virtual_price() * minStable) / 1e18;

3.9598229e32 // 32 zeroes
/
1e18

Gives us:
3.9598229e14

So for this condition
quoteAmount = (baseAmount * price) / 1e18;
Any baseAmount with less than 4 units will return a 0.

Hence we quantified the attack.

This is denominated in eth as well.

A gwei is 1e9

Because of EIP1559 there is no way to have a zero gas tx, I believe 1 gwei may be the minimum cost (but may be wrong)

This ultimately means that while the rounding is present there should be no economic incentive in exploiting it, as the MEV extractable has 4 order of magnitude but the minimum cost would have 9 (1 gwei = 10^9 wei)

At this time, given these considerations I believe the finding to be valid and to be of low severity

@GalloDaSballo GalloDaSballo added 1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Something isn't working sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

3 participants