Description

The VaultManager contract solely resides on the main chain and is tasked with tracking deposited funds within storage contracts on other chains. When a user deposits tokens on another chain, with the help of a cross-chain message, this contract is notified about it and increases the balance of the token in source chain. But if the user wants to withdraw funds, then this process will happen in two steps.

First, the contract will reduce the total balance of the token in the withdrawal chain as well as increase the frozen balance. Second, after a successful withdrawal of funds in the destination chain, a cross-chain message to complete the withdrawal will be dispatched. After receiving it, the contract will reduce the frozen balance by this amount and the process will be completed.

But since the contracts on different chains are not directly connected, it is impossible to guarantee the success of the withdrawal of funds and there may be situations when the cross-chain messages about the withdrawal of funds will not be executed. In this case, the contract should be able to restore the previous balance and unfreeze the funds.

function frozenBalance(bytes32 _tokenHash, uint256 _chainId, uint128 _deltaBalance) external override onlyLedger {
    tokenBalanceOnchain[_tokenHash][_chainId] -= _deltaBalance;
    tokenFrozenBalanceOnchain[_tokenHash][_chainId] += _deltaBalance;
}

function finishFrozenBalance(bytes32 _tokenHash, uint256 _chainId, uint128 _deltaBalance)
    external
    override
    onlyLedger
{
    tokenFrozenBalanceOnchain[_tokenHash][_chainId] -= _deltaBalance;
}

Impact

In case of unsuccessful delivery of a cross-chain message, for example, if a malicious user sends withdrawn messages with a zero-recipient address that cannot be executed, the balance of funds on the destination chain will be reduced, although in fact the balance of the contract will remain unchanged. But it will not be possible to restore the funds on the balance of the VaultManager contract.

Recommendations

We recommend adding a function that allows the owner of VaultManager to return frozen funds to the balance.

Remediation

This finding has been acknowledged by Orderly Network. Their official response is paraphrased below:

In our design, withdrawals are irreversible, initiated by the user's EIP712 signature and processed by the offline engine. Typically, the frozenBalance is non-reversible. However, considering exceptional scenarios, we acknowledge the potential need for a restore function and may implement it if necessary.