Steadefi

Summary

Users are not refunded the extra value sent to cover execution fees when they deposit non native tokens.

Vulnerability Details

• Users can deposit tokens in vaults by calling GMXVault.deposit payable function with the required deposit parameters (such as token, amount, minimum share amount, slippage & execution fees), then this function will invoke GMXDeposit.deposit with a msg.value to cover the transaction fees required by the GMX exchange router to add liquidity (3rd party contract).

• In GMXDeposit.deposit: various checks are made to ensure the sanity of the deposit parameters and the elligibility of the user to deposit,and to calculate the required tokenA & tokenB required to deposit in the GMX protocol.

• Then an AddLiquidityParams struct is prepared to send with the GMXWorker.addLiquidity where it will call the GMX exchange router to add liquidity.

• The execution fee is going to be extracted from the deposit params sent by the user (_alp.executionFee = dp.executionFee;); but it was noticed that there's no check on the dp.executionFee if it equals the msg.value sent by the user; so users are not going to be refunded if they sent msg.value greater than the required transaction fees.

• The same issue is spotted in GMXWithdraw.withdraw function when the user removes liquidity from GMX.

Impact

Users are not going to be refunded if they sent msg.value greater than the required transaction fees.

Proof of Concept

GMXDeposit.deposit function/L141

GMXWithdraw.withdraw function/L148

Tools Used

Manual Review.

Recommendations

Add a check to ensure that dp.executionFee == msg.value and refund the extra amount to the user, and if the refund is going to be done by the GMX router, then update the alp.executionFee & rlp.executionFee to be equal to the sent msg.value.