Loading arbitrary's contract data to memory allows grieffing attack

Summary

Incorrect handling of returned data will cost more gas to keepers if callback contracts return an extense amount of data for the keeper to handle in the catch statement.

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/callback/CallbackUtils.sol#L79

Vulnerability Detail

In the functions from the callbackUtils.sol file, when execution fails, those functions load the memory data returned by the callbackContract(), which user can input to their own malicious contract returning extense data inside the gasLimit to increase the cost of processing that data for the keeper.

function afterDepositExecution(bytes32 key, Deposit.Props memory deposit) internal {
    if (!isValidCallbackContract(deposit.callbackContract())) { return; }

    try IDepositCallbackReceiver(deposit.callbackContract()).afterDepositExecution{ gas: deposit.callbackGasLimit() }(key, deposit) {
    } catch (bytes memory reasonBytes) {
        (string memory reason, /* bool hasRevertMessage */) = ErrorUtils.getRevertMessage(reasonBytes);
        emit AfterDepositExecutionError(key, deposit, reason, reasonBytes);
    }
}

Therefore the attack is not profitable for the attacker but rather costs gas to the keeper. Because instead of reverting on failure, they are handling the error from the callbackContract() in the catch statement, loading the memory data, which costs gas.

Impact

Increase the gas cost for the keeper in failed transactions by loading extra data to memory that callbackUtils handles.

Code Snippet

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/callback/CallbackUtils.sol#L65-L67

Tool used

Manual Review

Recommendation

It is not a good practice to load data from arbitrary contracts because it opens the attack vector for grieffing attacks. Consider not loading that data and reverting on failure

Duplicate of #141

sherlock-audit / 2023-02-gmx-judging

0xAmanda - Loading arbitrary's contract data to memory allows grieffing attack #198