peakbolt - Lack of balance check for `requestWithdraw()`

[sherlock-admin]

peakbolt

medium

Lack of balance check for requestWithdraw()

Summary

JOJOExternal.requestWithdraw() is missing check on trader's current balance. This will allow the trader to bypass th withdrawal timelock by sending a large withdrawal request way ahead of time, causing a DoS to the perpetual trading system.

Vulnerability Detail

The purpose of requestWithdraw() is meant to be used as a timelock on withdrawals from JOJO system and to allow the off-chain matching engine to reduce balances before on-chain withdrawals.

However, the issue is that there is no balance check for requestWithdraw(), which means the user could utilize the withdrawal request for future deposits too.

When that happens, the trade could cause the off-chain matching engine's balance accounting to be de-sychronized from the on-chain state, leading to a DoS.

    function requestWithdraw(
        Types.State storage state,
        uint256 primaryAmount,
        uint256 secondaryAmount
    ) external {
        state.pendingPrimaryWithdraw[msg.sender] = primaryAmount;
        state.pendingSecondaryWithdraw[msg.sender] = secondaryAmount;
        state.withdrawExecutionTimestamp[msg.sender] =
            block.timestamp +
            state.withdrawTimeLock;
        emit RequestWithdraw(
            msg.sender,
            primaryAmount,
            secondaryAmount,
            state.withdrawExecutionTimestamp[msg.sender]
        );
    }

Imagine the scenario,

1. Attacker deposits 1,000 USDC into account and opens a trade.
2. Attacker sets withdrawal request to 10,000 USDC using requestWithdraw().
3. Off-chain matching engine reduces balance to zero (or encounter error) and cancel Alice's trade.
4. Attacker deposits 9,000 USDC and opens a new trade that has the best price and long expiration, then calls executeWithdraw() to withdraw 10,000 USDC immediately, bypassing the timelock.
5. Off-chain matching engine will be de-synchronized from on-chain balance and will continue to match Attacker's orders, but the orders will fail to be settled on-chain due to insufficient balance.

Impact

Attacker is able to bypass the timelock and cause off-chain matching engine to be de-synchronized and DoS by causing it to process the attacker's invalid trade orders. This will cause legitimate traders to experience trading loss as their trades are denied and not executed in a timely manner.

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/impl/JOJOExternal.sol#L35-L40 https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/lib/Funding.sol#L84-L100

Tool used

Manual review

Recommendation

Check the trader's balance on requestWithdraw().

Duplicate of #429