sherlock-audit / 2023-04-gmx-judging

2 stars 1 forks source link

Jaraxxus - Malicious keeper can still DoS deposits and gain rewards using the 63/64 rule #276

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Jaraxxus

medium

Malicious keeper can still DoS deposits and gain rewards using the 63/64 rule

Summary

Keepers can call _executeDeposit with 63/64 of the gas needed in order to avoid executing deposit and invoking _handleDepositError instead to receive rewards, if _handleDepositError passes with 1/64 of the total gas.

Vulnerability Detail

A malicious keeper can call executeDeposit and use only 63/64 of the required gas to that the _handleDepositError in the catch statement will be called instead.

File: gmx-synthetics\contracts\exchange\DepositHandler.sol
092:     function executeDeposit(
093:         bytes32 key,
094:         OracleUtils.SetPricesParams calldata oracleParams
095:     ) external
096:         globalNonReentrant
097:         onlyOrderKeeper
098:         withOraclePrices(oracle, dataStore, eventEmitter, oracleParams)
099:     {
100:         uint256 startingGas = gasleft();
101: 
102:@>       try this._executeDeposit(
103:             key,
104:             oracleParams,
105:             msg.sender
106:         ) {
107:@>       } catch (bytes memory reasonBytes) {
108:             _handleDepositError(
109:                 key,
110:                 startingGas,
111:                 reasonBytes
112:             );
113:         }
114:     }

Impact

Malicious keeper can get funds by purposely denying execution

Code Snippet

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/exchange/DepositHandler.sol#L92-L114

Tool used

Manual Review

Recommendation

Add additional checks to make sure that keeper doesn't use 63/64 of the gas to purposely invoke the _handleDepositError function.

Related to M-31.