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

No slippage protection in VaderPoolV2.mintSynth #164

Closed
code423n4 opened this issue Nov 15, 2021 · 1 comment
Closed

No slippage protection in VaderPoolV2.mintSynth #164

code423n4 opened this issue Nov 15, 2021 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists VaderPoolV2

Comments

@code423n4
Copy link
Contributor

Handle

cmichel

Vulnerability details

The VaderPoolV2.mintSynth implicitly performs a "native -> foreign" swap using VaderMath.calculateSwap(nativeDeposit,reserveNative,reserveForeign), the resulting amount will be minted as synths instead of transferred out as foreign tokens.

The calculateSwap calculation, therefore, depends on the current reserves and the mintSynth does not allow specifying any minSynthAmount for the caller to make sure that they cannot be sandwich attacked.
It's basically doing a trade with a slippage check always set to 100%.

Impact

Any mintSynth call can be sandwich attacked to make a profit.

A common attack in DeFi is the sandwich attack. Upon observing a trade of asset X for asset Y, an attacker frontruns the victim trade by also buying asset Y, lets the victim execute the trade, and then backruns (executes after) the victim by trading back the amount gained in the first trade. Intuitively, one uses the knowledge that someone’s going to buy an asset, and that this trade will increase its price, to make a profit. The attacker’s plan is to buy this asset cheap, let the victim buy at an increased price, and then sell the received amount again at a higher price afterwards.

  • Attacker frontruns mintSynth by performing a standard swap, trading native assets to foreign assets, leading to a worse synth price for the victim
  • Victim performs the mint synth, depositing native tokens (receiving synths at a worse price)
  • Attacker now sells the received foreign assets from the first step into the increased native reserves for a profit

Trades can happen at a bad price and lead to receiving fewer tokens than at a fair market price.
The attacker's profit is the user's loss.

Recommended Mitigation Steps

Add minimum return amount checks.
Accept a function parameter that can be chosen by the transaction sender, then check that the actually received amount is above this parameter.

@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 Nov 15, 2021
code423n4 added a commit that referenced this issue Nov 15, 2021
@SamSteinGG SamSteinGG added the duplicate This issue or pull request already exists label Nov 25, 2021
@SamSteinGG
Copy link
Collaborator

Duplicate #2

@alcueca alcueca added 3 (High Risk) Assets can be stolen/lost/compromised directly and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Dec 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists VaderPoolV2
Projects
None yet
Development

No branches or pull requests

4 participants