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

VaderReserve.reimburseImpermanentLoss improperly converts USDV to VADER #7

Open
code423n4 opened this issue Dec 21, 2021 · 0 comments
Labels
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 sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Handle

TomFrenchBlockchain

Vulnerability details

Impact

IL isn't properly converted from being in terms of USDV to VADER, resulting in reserve paying out incorrect amount.

Proof of Concept

VaderReserve.reimburseImpermanentLoss receives an amount in terms of USDV and converts this to an amount of VADER to send to recipient.

However as shown in the link if there is a previous price stored for USDV, the amount of VADER tokens to be sent to the recipient is amount / usdvPrice.

https://github.com/code-423n4/2021-12-vader/blob/fd2787013608438beae361ce1bb6d9ffba466c45/contracts/reserve/VaderReserve.sol#L84-L110

usdvPrice is the total USD value of foreign assets divided by the total amount of USDV in a number of pairs. It's then some measure of the inverse of the price of USDV in USD, nothing to do with converting into VADER.

The reserve will then improperly calculate the amount of VADER to pay out once there is a single reading of the USDV price.

Recommended Mitigation Steps

It looks like both branches of this if statement are supposed to be run, i.e. convert from USDV to USD and then to VADER but I can't be sure. Care should be taken so that the calculation being performed is the expected one.

@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 Dec 21, 2021
code423n4 added a commit that referenced this issue Dec 21, 2021
@SamSteinGG SamSteinGG added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 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

2 participants