stopthecap - Underestimated gas estimation for executing withdrawals leads to insufficient keeper compensation

[sherlock-admin]

stopthecap

high

Underestimated gas estimation for executing withdrawals leads to insufficient keeper compensation

Summary

The GasUtils.estimateExecuteWithdrawalGasLimit function underestimates the gas estimation for withdrawal execution, as it does not take into account the param shouldUnwrapNativeToken, unlike the gas estimation in the GasUtils.estimateExecuteDepositGasLimit function (used to estimate executing deposits).

Vulnerability Detail

When creating a withdrawal request, the WithdrawalUtils.createWithdrawal function estimates the gas required to execute the withdrawal and validates that the paid execution fee (params.executionFee) is sufficient to cover the estimated gas and to compensate the keeper executing the withdrawal fairly.

However, the GasUtils.estimateExecuteWithdrawalGasLimit function used to estimate the gas for executing withdrawals does not account for token wrapping/unwrapping that occurs at the end of the withdrawal logic and therefore underestimates the gas estimation.

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/swap/SwapUtils.sol#L111

Impact

The keeper executing withdrawals receives fewer execution fees and is not fully compensated for the gas spent if shouldUnwrapNativeToken is true. Moreover, users can pay fewer execution fees than expected and required.

Code Snippet

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/gas/GasUtils.sol#L143-L149

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/withdrawal/WithdrawalUtils.sol#L150

Tool used

Manual Review

Recommendation

Consider incorporating the extra gas spent from unwrapping WETH in the gas estimation for withdrawal execution.

[0xffff11]

Escalate for 10 USDC.

The issue is valid, perhaps a medium instead of a high. Miss-calculations of gas have been awarded as medium and high in both gmx contests:

Second contest: #199
First contest: sherlock-audit/2023-02-gmx-judging#189

The non calculation of 1/64 is a small non-calculation which is awarded as a medium. This issue reflects the non calculation of the gas spent to unwrap the native token when selected. Therefore it is a non calculation of gas in a 1 of 2 case. Similarly, the sherlock-audit/2023-02-gmx-judging#189 issue is also a non-calculation in a swapping tokens edge-case.

For those, reasons, I believe that the issue should be a valid medium. It is a valid non calculation of gas. Given 2 previous examples and stated similarities to both.

Edit: Also found: #222 It should be a duplicate of this one.

[sherlock-admin]

Escalate for 10 USDC.

The issue is valid, perhaps a medium instead of a high. Miss-calculations of gas have been awarded as medium and high in both gmx contests:

Second contest: #199
First contest: sherlock-audit/2023-02-gmx-judging#189

The non calculation of 1/64 is a small non-calculation which is awarded as a medium. This issue reflects the non calculation of the gas spent to unwrap the native token when selected. Therefore it is a non calculation of gas in a 1 of 2 case. Similarly, the sherlock-audit/2023-02-gmx-judging#189 issue is also a non-calculation in a swapping tokens edge-case.

For those, reasons, I believe that the issue should be a valid medium. It is a valid non calculation of gas. Given 2 previous examples and stated similarities to both.

Edit: Also found: #222 It should be a duplicate of this one.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[IllIllI000]

It is part of the base fee as is described in the other escalation. Low

[hrishibhat]

Result:
Invalid
Unique
Pasting the Lead Watson comment from the escalation in #222

This is part of the base fee of the order. The code block above even states this:

        // additionally, a transaction could fail midway through an execution transaction
        // before being cancelled, the possibility of this additional gas cost should
        // be considered when setting the baseGasLimit

Invalid

Considering this a non-issue

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• 0xffff11: rejected