sherlock-audit / 2023-02-gmx-judging

17 stars 11 forks source link

simon135 - slippage set to 0 it can cause users to get sandwiched #211

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

simon135

high

slippage set to 0 it can cause users to get sandwiched

Summary

If the user wants to decrease their position and want to swap 10 eth to their collateral token but slippage is set to 0 so they will get a bad price and can lose alot of their eth

Vulnerability Detail

ex: bob has pos and loses 1 eth and is left with 10 eth they want to decrease the pos and get their collateral back in usdc they will have to swap with slippage set to 0 so they will lose funds it's even worse the markets get the funds back they won't be able to pay out the full amount and they will be in dos state until the protocol is made whole

Impact

no slippage check

Code Snippet


     try params.contracts.swapHandler.swap(
                SwapUtils.SwapParams(
                    params.contracts.dataStore,
                    params.contracts.eventEmitter,
                    params.contracts.oracle,
                    Bank(payable(params.market.marketToken)),
                    pnlToken, // tokenIn
                    profitAmount, // amountIn
                    swapPathMarkets, // markets
                    0, // minOutputAmount
                    params.market.marketToken, // receiver
                    false // shouldUnwrapNativeToken

Tool used

Manual Review

Recommendation

add slippage checks and user validate min amount checks

Duplicate of #138