If account is blacklisted by usdc/usdt then the order the will revert and cant get switched

Summary

If an account is blacklisted by usdc/usdt then the order they will revert and can't get switched

Vulnerability Detail

The receiver can also be blacklisted but the user can change it. But for canceling an order we can't change the account address and if the account address gets blacklisted and the order cant go through and the cancel order will also revert the user wont be able to get their funds

Impact

funds loss possible or dos

Code Snippet

        if (order.initialCollateralDeltaAmount() > 0) {
                orderVault.transferOut(
                    order.initialCollateralToken(),
                    order.account(),
                    order.initialCollateralDeltaAmount(),
                    order.shouldUnwrapNativeToken()
                );
            }
        }

Tool used

Manual Review

Recommendation

Make a way for account to be switched to different address

snn20 commented 1 year ago

escalate for 10 USDC If the account gets blacklisted and the account requests a deposit at t0 (time 0), the account would get back 5 market tokens but the user doesn't want that rate and wants 10 market tokens so the user wants to cancel the deposit so when the user tries to cancel the deposit it will revert because they are blacklisted so the deposit will go through when the user gets a bad rate they won't be able to and they will get a bad rate I think this deserves medium severity like it was judged in other contests ex: 1.Alice creates a request for depositing 5 USDC rate = 5 market tokens

Alice gets blacklisted from usdc

Alice doesn't want the bad rate so she tries to cancel the deposit but she cant and the tx revert


    if (deposit.initialLongTokenAmount() > 0) {
        depositVault.transferOut(
            deposit.initialLongToken(),
            deposit.account(),
            deposit.initialLongTokenAmount(),
            deposit.shouldUnwrapNativeToken()
        );
    }

    if (deposit.initialShortTokenAmount() > 0) {
        depositVault.transferOut(
            deposit.initialShortToken(),
            deposit.account(),
            deposit.initialShortTokenAmount(),
            deposit.shouldUnwrapNativeToken()
        );
    }


https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/deposit/DepositUtils.sol#L162-L177
#### Impact
just imagine  if an attacker can cause a blacklist on an account and cause a bad rate  
the plain simple attack here is dos and gas lost from the user
The Functionality is broken since an account cant cancel a deposit even if they wanted to

sherlock-admin commented 1 year ago

escalate for 10 USDC If the account gets blacklisted and the account requests a deposit at t0 (time 0), the account would get back 5 market tokens but the user doesn't want that rate and wants 10 market tokens so the user wants to cancel the deposit so when the user tries to cancel the deposit it will revert because they are blacklisted so the deposit will go through when the user gets a bad rate they won't be able to and they will get a bad rate I think this deserves medium severity like it was judged in other contests ex: 1.Alice creates a request for depositing 5 USDC rate = 5 market tokens
Alice gets blacklisted from usdc
Alice doesn't want the bad rate so she tries to cancel the deposit but she cant and the tx revert
    if (deposit.initialLongTokenAmount() > 0) {
        depositVault.transferOut(
            deposit.initialLongToken(),
            deposit.account(),
            deposit.initialLongTokenAmount(),
            deposit.shouldUnwrapNativeToken()
        );
    }

    if (deposit.initialShortTokenAmount() > 0) {
        depositVault.transferOut(
            deposit.initialShortToken(),
            deposit.account(),
            deposit.initialShortTokenAmount(),
            deposit.shouldUnwrapNativeToken()
        );
    }
https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/deposit/DepositUtils.sol#L162-L177
#### Impact
just imagine  if an attacker can cause a blacklist on an account and cause a bad rate  
the plain simple attack here is dos and gas lost from the user
The Functionality is broken since an account cant cancel a deposit even if they wanted to

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.

IllIllI000 commented 1 year ago

The code block this issue points to is only for sending back collateral that was provided with the order to modify the position, so the issue being pointed to is that a user provides collateral, but since they're blocklisted they can't get their own collateral back. Sherlock does not consider simple blocklisting issues such as this as valid - Invalid

hrishibhat commented 1 year ago

Escalation rejected

User Blacklisting resulting in loss of funds for themselves is not a valid issue.

sherlock-admin commented 1 year ago

Escalation rejected

User Blacklisting resulting in loss of funds for themselves is not a valid issue.

This issue's escalations have been rejected!

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

sherlock-audit / 2023-02-gmx-judging

simon135 - If account is blacklisted by usdc/usdt then the order the will revert and cant get switched #229