Additional checks to be performed

Below is a list of additional checks that could be performed. As of right now, they do not pose direct security implications, but we believe that they could be beneficial for the project in the long run.

Assure that account.frozenBalances is properly updated

In AccountTypeHelper, the account.frozenBalances is set to 0 instead of subtracting the amount that was withdrawn. This could lead to a situation where the user withdraws more than they have in their frozen balance.

function finishFrozenBalance(
    AccountTypes.Account storage account,
    uint64 withdrawNonce,
    bytes32 tokenHash,
    uint128 amount
) internal {
    account.totalFrozenBalances[tokenHash] -= amount;
-   account.frozenBalances[withdrawNonce][tokenHash] = 0;
+   account.frozenBalances[withdrawNonce][tokenHash] -= amount;
}

Remediation

This issue has been acknowledged by Orderly Network, and a fix was implemented in commit e6557dcb↗.

The same issue occurs in unfrozenBalance.

Several missing checks in the Ledger contract

The function accountDeposit does not validate the data.srcChainDepositNonce. The verification of this value will ensure that deposits are coming in the correct order.

The function executeWithdrawAction does not check that the input withdraw.receiver address is not a zero address. The lack of this check may lead to the problem that this cross-chain message cannot be delivered successfully, which leads to blocking the transmission of cross-chain messages.

There are lack of checks that the input ledgerExecution.symbolHash and Adl.symbolHash are allowed in the executeSettlement and executeAdl functions.

The functions executeLiquidation, executeSettlement, and executeAdl do not validate directly that perpetual position for the user account exists.

There is a lack of validation that the ledgerExecution.settledAmount.abs() value is not greater than position.costPosition.abs() in the executeSettlement function.

The _feeSwapPosition function increases the traderPosition.costPosition by feeAmount, which always is positive. But the function does not take into account the sign of costPosition, and this can lead to an issue if costPosition is negative. Then, the feeAmount reduces the absolute value of costPosition.

function _feeSwapPosition(
    AccountTypes.PerpPosition storage traderPosition,
    bytes32 symbol,
    uint128 feeAmount,
    uint64 tradeId,
    int128 sumUnitaryFundings
) internal {
    if (feeAmount == 0) return;
    _perpFeeCollectorDeposit(symbol, feeAmount, tradeId, sumUnitaryFundings);
    traderPosition.costPosition += feeAmount.toInt128();
}

Remediation

This issue has been acknowledged by Orderly Network, and a fix was implemented in commit 08e402b0↗.