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

0xAmanda - Uncalculated gas while swapping tokens will make the keeper lose funds on withdrawals #189

Closed
sherlock-admin opened this issue Mar 25, 2023 · 0 comments
Labels
Duplicate High Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 25, 2023

0xAmanda

high

Uncalculated gas while swapping tokens will make the keeper lose funds on withdrawals

Summary

While calculating the gas that is estimated for withdrawals, it does not calculate also the gas of the swapPath for none of both tokens, long and short. Those paths are addresses from the struct CreateWithdrawalParams used for creating a withdrawal.

Vulnerability Detail

The following struct is used with creating a withdrawal order:

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

   struct CreateWithdrawalParams {
    address receiver;
    address callbackContract;
    address market;
    address[] longTokenSwapPath;
    address[] shortTokenSwapPath;
    uint256 marketTokenAmount;
    uint256 minLongTokenAmount;
    uint256 minShortTokenAmount;
    bool shouldUnwrapNativeToken;
    uint256 executionFee;
    uint256 callbackGasLimit;
   }

The following 2 params:

    address[] longTokenSwapPath;
    address[] shortTokenSwapPath;

are not accounted in the calculation of the gas to execute the tx.

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

Calculation using the withdrawal request:

    uint256 estimatedGasLimit = GasUtils.estimateExecuteWithdrawalGasLimit(dataStore, withdrawal);

As you can see in the previous snippets, there is no calculation or whatsoever of the gas that will cost swapping the tokens through the previously inputted paths. Specially if the path is long, the gas used for swapping will compound charging the keeper because they were unexpected costs. These gas will be deducted directly from the contract, draining it slowly due to the missed calculation

Impact

High. The contract (the keeper)will be charged the gas that is not accounted for those swapPaths, draining the contracts native currency slowly.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add a gas calculation for the swapPaths of both, the long and short token. Consider calculating that amount taking into account the lenght of each path, because that will increase/decrease the gas costs.

Duplicate of #195

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate High Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant