sherlock-audit/2023-02-gmx-judging

hack3r-0m - callback receiver can control when to allow order execution

Closed this issue · 5 comments

hack3r-0m

high

callback receiver can control when to allow order execution

Summary

callback receiver can control when to allow order execution

Vulnerability Detail

        uint256 marketTokensBalance = MarketToken(payable(withdrawal.market())).balanceOf(withdrawal.account());
        if (marketTokensBalance < withdrawal.marketTokenAmount()) {
            revert InsufficientMarketTokens(marketTokensBalance, withdrawal.marketTokenAmount());
        }

this allow attackers to have control over when it will not be reverted, attacker will increase upto required balance to and delay withdrawal to benifit from fees or market movement until that

Impact

unintended access control for when to allow withdrawal

Code Snippet

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/deposit/ExecuteDepositUtils.sol#L392

Tool used

Manual Review

Recommendation

do not use balanceOf checks, instead revert and consume all gas on failed transfer

Escalate for 10 USDC
here, the callback receiver can control value balanceOf(marketToken) (practically, by transfering to some other address then itself and so check fails and withdrawal doesn't execute) which allows risk free trade.

the code snippet in "Vulnerability Detail" is the correct one i wanted to mention while location on code snippet location should be https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/withdrawal/WithdrawalUtils.sol#L195 , apologies in case judges were not to find that and marked issue excluded

Escalate for 10 USDC
here, the callback receiver can control value balanceOf(marketToken) (practically, by transfering to some other address then itself and so check fails and withdrawal doesn't execute) which allows risk free trade.

the code snippet in "Vulnerability Detail" is the correct one i wanted to mention while location on code snippet location should be https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/withdrawal/WithdrawalUtils.sol#L195 , apologies in case judges were not to find that and marked issue excluded

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.

executeWithdrawal(), which throws the error, is called by _executeWithdrawal(), which, if it throws an exception, will cancel the order - Invalid

Escalation rejected

Based on the comments above there is no valid issue here

Escalation rejected

Based on the comments above there is no valid issue here

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.