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

BaseVaultAdaptor assumes sharePrice is always in underlying decimals #114

Open
code423n4 opened this issue Jul 7, 2021 · 0 comments
Open
Labels

Comments

@code423n4
Copy link
Contributor

Handle

cmichel

Vulnerability details

Vulnerability Details

The two BaseVaultAdaptor.calculateShare functions computes share = amount.mul(uint256(10)**decimals).div(sharePrice)

uint256 sharePrice = _getVaultSharePrice();
// amount is in "token" decimals, share should be in "vault" decimals
share = amount.mul(uint256(10)**decimals).div(sharePrice);

This assumes that the sharePrice is always in token decimals and that token decimals is the same as vault decimals.

This both happens to be the case for Yearn vaults, but will not necessarily be the case for other protocols.
As this functionality is in the BaseVaultAdaptor and not in the specific VaultAdaptorYearnV2_032, consider generalizing the conversion.

Impact

Integrating a token where the token or price is reported in a different precision will lead to potential losses as more shares are computed.

Recommended Mitigation Steps

The conversion seems highly protocol specific, calculateShare should be an abstract function like _getVaultSharePrice, that is implemented in the specific adaptors.

@code423n4 code423n4 added 2 (Med Risk) bug Something isn't working labels Jul 7, 2021
code423n4 added a commit that referenced this issue Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants