sherlock-audit/2023-02-gmx-judging

IllIllI - Fee receiver does not get paid when collateral is enough to cover the funding fee, during liquidation

Opened this issue · 2 comments

IllIllI

medium

Fee receiver does not get paid when collateral is enough to cover the funding fee, during liquidation

Summary

When a position is liquidated, and there is enough collateral to cover the outstanding funding fee, the fee receiver does not get paid its fee

Vulnerability Detail

All of the remaining collateral is given to the pool, rather than paying the portion of position fees owed to the fee receiver

Impact

Fee receiver does not get paid its share, which is instead given to the pool

Code Snippet

The PnL funds are all given to the pool, and the _fees variable, which contains the fee receiver portion remains uninitialized.

// File: gmx-synthetics/contracts/position/DecreasePositionCollateralUtils.sol : DecreasePositionCollateralUtils.getLiquidationValues()   #1

346            } else {
347 @>             values.pnlAmountForPool = (params.position.collateralAmount() - fees.funding.fundingFeeAmount).toInt256();
348            }
349    
350 @>         PositionPricingUtils.PositionFees memory _fees;
351    
352            PositionUtils.DecreasePositionCollateralValues memory _values = PositionUtils.DecreasePositionCollateralValues(
353                values.pnlTokenForPool,
354                values.executionPrice, // executionPrice
355                0, // remainingCollateralAmount
356                values.positionPnlUsd, // positionPnlUsd
357 @>             values.pnlAmountForPool, // pnlAmountForPool
358                0, // pnlAmountForUser
359                values.sizeDeltaInTokens, // sizeDeltaInTokens
360                values.priceImpactAmount, // priceImpactAmount
361                0, // priceImpactDiffUsd
362                0, // priceImpactDiffAmount
363                PositionUtils.DecreasePositionCollateralValuesOutput(
364                    address(0),
365                    0,
366                    address(0),
367                    0
368                )
369            );
370    
371 @>         return (_values, _fees);
372:       }

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/position/DecreasePositionCollateralUtils.sol#L342-L372

The empty fees are returned, and the function exits, skipping the paying of the fee receiver's portion of the position fees that are owed.

Tool used

Manual Review

Recommendation

Split the fees properly in getLiquidationValues() and don't return early after the call to getLiquidationValues()

xvi10 commented

this is a valid concern, but we do not think that the contracts should be changed for it, for simplicity the fee receiver does not receive a fee for this case

I'm leaving this open since the fees are not paid