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

H-03 MitigationConfirmed #36

Open
code423n4 opened this issue Aug 22, 2023 · 1 comment
Open

H-03 MitigationConfirmed #36

code423n4 opened this issue Aug 22, 2023 · 1 comment
Labels
edited-by-warden mitigation-confirmed MR-H-03 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

code423n4 commented Aug 22, 2023

Lines of code

Vulnerability details

Original Issue

H-03 - _amountOut is representing assets and shares at the same time in the liquidate function

Details

In the previous implementation of the Vault contract it was used an exchangeRate to determine the number of assets/shares to be minted/burned/transferred, this means that when a value was in terms of asset units but the operation to be executed needed the value to be in shares units, the original value that was in asset units needed to be converted to shares units before executing the operation.
The same reasoning applies when comparing asset units against share units, because of the difference that existed between them, if asset units were compared against share units, the comparisson was not correct, it was like comparing apples against peaches.

Mitigation

The mitigation was to refactor the way how the Vault determines if it's collateralized or not, as part of this change, the exchangeRate was removed, and instead new logic was implemented to make that the shares are fully backed 1:1 to assets in the YieldVault, which means that now, there is no difference if the value is in terms of assets or shares, they are the same, so, there is no need to convert assets to shares or shares to assets.

Conclusion

Because of the refactoring and the removal of the concept of the exchangeRate and the introduction of shares fully backed up 1:1 with assets, the original issue is correctly mitigated, and now there are no problems using asset units mixed with share units.

  • The collateralization state of the vault is checked when calculating the liquidableYield, if the vault is collateralized, the Yield is the difference between all the assets handled by the YieldVault on behalf of the Vault and all the shares minted in the Vault (which represents all the deposited assets in the Vault).
@c4-judge
Copy link

c4-judge commented Sep 5, 2023

Picodes marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
edited-by-warden mitigation-confirmed MR-H-03 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants