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

X12 - WStethRatiosAggregatorV3 will not work properly if the base token is anything but stETH #29

Closed
sherlock-admin4 opened this issue Jun 27, 2024 · 4 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Jun 27, 2024

X12

Medium

WStethRatiosAggregatorV3 will not work properly if the base token is anything but stETH

Summary

Vaults have impermanent loss because they rely on the current price.

Vulnerability Detail

The initial base token for our system is WETH, as seen here.

baseTokens - Returns the base token associated with a specific vault. - baseTokens[Vault Address] = Wrapped ETH Address

With that in mind, when we extract the price from WStethRatiosAggregatorV3, our latestRoundData will return IWSteth(wsteth).getStETHByWstETH, which is the price of wstETH in stETH terms based on Lido's docs.

However, WETH : stETH can have small price differences (i.e., it's not 1 : 1), and the ratio of wstETH in stETH will result in slight price differences when used with our base token - WETH. This will cause the real price of wstETH to be over or undervalued, resulting in users depositing fewer or more shares than they should.

Note that even if the base token is something else (other than stETH), the same issue will appear.

Impact

Slight price differences can cause inaccurate share allocations.

Code Snippet

function getAnswer() public view returns (int256) {
    return int256(IWSteth(wsteth).getStETHByWstETH(10 ** decimals));
}

function latestRoundData() public view override returns (uint80, int256, uint256, uint256, uint80) {
    return (0, getAnswer(), block.timestamp, block.timestamp, 0);
}

Tool used

Manual Review

Recommendation

Have WStethRatiosAggregatorV3 perform another conversion for stETH to the base token and use that price.

Duplicate of #266

@sherlock-admin3 sherlock-admin3 changed the title Creamy Malachite Condor - Operators are not paid for executing TXs WStethRatiosAggregatorV3 will not work properly if the base token is anything but stETH Jun 28, 2024
@sherlock-admin3 sherlock-admin3 added the Sponsor Disputed The sponsor disputed this issue's validity label Jun 30, 2024
@github-actions github-actions bot changed the title WStethRatiosAggregatorV3 will not work properly if the base token is anything but stETH Creamy Malachite Condor - WStethRatiosAggregatorV3 will not work properly if the base token is anything but stETH Jul 6, 2024
@github-actions github-actions bot closed this as completed Jul 6, 2024
@github-actions github-actions bot added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Excluded Excluded by the judge without consulting the protocol or the senior labels Jul 6, 2024
@z3s
Copy link
Collaborator

z3s commented Jul 12, 2024

Invalid; IWSteth(wsteth).getStETHByWstETH(10 ** decimals) returns correct ratio!?

Edit: Ignore this comment

@sherlock-admin3 sherlock-admin3 changed the title Creamy Malachite Condor - WStethRatiosAggregatorV3 will not work properly if the base token is anything but stETH X12 - WStethRatiosAggregatorV3 will not work properly if the base token is anything but stETH Jul 15, 2024
@sherlock-admin3 sherlock-admin3 added Non-Reward This issue will not receive a payout and removed Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Jul 15, 2024
@ADK0010
Copy link

ADK0010 commented Jul 15, 2024

I think the above comment by @z3s is due to my report . I believe my report is under the name "Angry_Mustache_Man" has been wrongly duped with these set of issues . I don't know whether this family of issues are completely correct or not ,but the issue #228 I have submitted is wrong and for it the above comment by @z3s is valid .

@IlIlHunterlIlI
Copy link

protocol in discord we assume 1 steth == 1 weth, at least for current deployments, does it implies on this?

@WangSecurity
Copy link

I see the comment from the sponsor, but the problem is that this issue can cause a loss of funds to users and this assumption is not mentioned in the README or code comments, I believe it should be valid and a duplicate of #266 as explained here

@WangSecurity WangSecurity added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label and removed Excluded Excluded by the judge without consulting the protocol or the senior labels Jul 30, 2024
@sherlock-admin4 sherlock-admin4 added Reward A payout will be made for this issue and removed Non-Reward This issue will not receive a payout labels Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity
Projects
None yet
Development

No branches or pull requests

6 participants