Incomplete error handling causes execution and freezing/cancelling of Deposits/Withdrawals/Orders to fail.
Users can define callbacks for Deposits/Withdrawals/Orders execution and cancellations. GMX protocol attempts to manage errors during the execution of the callbacks\\nA user controlled callback can return a specially crafted revert reason that will make the error handling revert.\\nBy making the execution and cancelation revert, a malicious actor can game orders and waste keeper gas.\\nThe bug resides in ErrorUtilss `getRevertMessage` that is called on every callback attempt. Example of deposit callback:\\n```\\ntry IDepositCallbackReceiver(deposit.callbackContract()).afterDepositExecution{ gas: deposit.callbackGasLimit() }(key, deposit) {\\n        } catch (bytes memory reasonBytes) {\\n            (string memory reason, /* bool hasRevertMessage */) = ErrorUtils.getRevertMessage(reasonBytes);\\n            emit AfterDepositExecutionError(key, deposit, reason, reasonBytes);\\n        }\\n```\\n\\nAs can be seen in the above above snippets, the `reasonBytes` from the catch statement is passed to `getRevertMessage` which tries to extract the `Error(string)` message from the revert. The issue is that the data extracted from the revert can be crafted to revert on `abi.decode`.\\nI will elaborate: Correct (expected) revert data looks as follows: 1st 32 bytes: 0x000..64 (bytes memory size) 2nd 32 bytes: 0x08c379a0 (Error(string) selector) 3rd 32 bytes: offset to data 4th 32 bytes: length of data 5th 32 bytes: data\\n`abi.decode` reverts if the data is not structure correctly. There can be two reasons for revert:\\nif the 3rd 32 bytes (offset to data) is larger then the uint64 (0xffffffffffffffff)\\nSimplified yul: `if gt(offset, 0xffffffffffffffff) { revert }`\\nif the 3rd 32 bytes (offset to data) is larger then the uint64 of the encoded data, the call will revert\\nSimplieifed yul: `if iszero(slt(add(offset, 0x1f), size) { revert }`\\nBy reverting with the following data in the callback, the `getRevertMessage` will revert: 0x000....64 0x0x08c379a0...000 0xffffffffffffffff....000 0x000...2 0x4141
When parsing the revert reason, validate the offsets are smaller then the length of the encoding.
There are two impacts will occur when the error handling reverts:\\n(1) Orders can be gamed\\nSince the following callbacks are controlled by the user,:\\n`afterOrderExecution`\\n`afterOrderCancellation`\\n`afterOrderFrozen`\\nThe user can decide when to send the malformed revert data and when not. Essentially preventing keepers from freezing orders and from executing orders until it fits the attacker.\\nThere are two ways to game the orders:\\nAn attacker can create a risk free order, by setting a long increase order. If the market increases in his favor, he can decide to "unblock" the execution and receive profit. If the market decreases, he can cancel the order or wait for the right timing.\\nAn attacker can create a limit order with a size larger then what is available in the pool. The attacker waits for the price to hit and then deposit into the pool to make the transaction work. This method is supposed to be prevented by freezing orders, but since the attacker can make the `freezeOrder` revert, the scenario becomes vulnerable again.\\n(2) drain keepers funds\\nSince exploiting the bug for both execution and cancellation, keepers will ALWAYS revert when trying to execute Deposits/Withdrawals/Orders. The protocol promises to always pay keepers at-least the execution cost. By making the execution and cancellations revert the Deposits/Withdrawals/Orders will never be removed from the store and keepers transactions will keep reverting until potentially all their funds are wasted.
```\\ntry IDepositCallbackReceiver(deposit.callbackContract()).afterDepositExecution{ gas: deposit.callbackGasLimit() }(key, deposit) {\\n        } catch (bytes memory reasonBytes) {\\n            (string memory reason, /* bool hasRevertMessage */) = ErrorUtils.getRevertMessage(reasonBytes);\\n            emit AfterDepositExecutionError(key, deposit, reason, reasonBytes);\\n        }\\n```\\n