Skip to content
This repository has been archived by the owner on Dec 10, 2023. It is now read-only.

IllIllI - Virtual swap impacts can be bypassed by swapping through markets where only one of the collateral tokens has virtual inventory #257

Open
sherlock-admin opened this issue Jun 5, 2023 · 1 comment
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

IllIllI

medium

Virtual swap impacts can be bypassed by swapping through markets where only one of the collateral tokens has virtual inventory

Summary

Virtual swap impacts can be bypassed by swapping through markets where only one of the collateral tokens has virtual inventory

Vulnerability Detail

The code that calculates price impacts related to swapping, skips the application of virtual impacts if one of the tokens doesn't have a virtual token set

Impact

If the virtual swap amount for a particular token is very large, and a large swap through that market would cause the balance to drop a lot, causing the trade to have a large negative impact, a user can split their large order into multiple smaller orders, and route them through other markets where there is no virtual token for one of the pools, and avoid the fees (assuming those pools have non-virtual imbalances that favor such a trade).

Code Snippet

Virtual impacts are completely skipped if one of the tokens doesn't have a virtual version:

// File: gmx-synthetics/contracts/pricing/SwapPricingUtils.sol : SwapPricingUtils.getPriceImpactUsd()   #1

113            (bool hasVirtualInventoryTokenB, uint256 virtualPoolAmountForTokenB) = MarketUtils.getVirtualInventoryForSwaps(
114                params.dataStore,
115                params.market.marketToken,
116                params.tokenB
117            );
118    
119            if (!hasVirtualInventoryTokenA || !hasVirtualInventoryTokenB) {
120 @>             return priceImpactUsd;
121:           }

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/pricing/SwapPricingUtils.sol#L113-L121

Tool used

Manual Review

Recommendation

Use the non-virtual token's inventory as the standin for the missing virtual inventory token

@github-actions github-actions bot added the Medium A valid Medium severity issue label Jun 9, 2023
@xvi10 xvi10 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Jun 13, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Jun 20, 2023
@xvi10
Copy link

xvi10 commented Jul 5, 2023

fixed in gmx-io/gmx-synthetics@1b35173

virtual swap price impact should be tracked by virtualInventoryForSwapsKey(virtualMarketId, isLongToken) instead of virtualInventoryForSwapsKey(virtualMarketId, token)

this would allow similar markets e.g. ETH/USDC, ETH/USDT to be associated together

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

2 participants