CCDMClient Manual Review Findings

CCM-01M: Inexistent Initialization Protection of Base Implementation

Type	Severity	Location
Language Specific		CCDMClient.sol:L59

Description:

The contract is meant to be upgradeable yet does not properly protect its logic deployment from malicious initializations.

Example:

Copy
49contract CCDMClient is IMessageRecipient, MailboxClient, CCDMClientStrorageV1 {
50    using Conv for address;
51    using Conv for bytes32;
52
53    event Adjusted(address indexed executor, bytes32 indexed l1Asset, address indexed l2Asset, uint256 resolved);
54    event DepositSuccess(address indexed receiver, bytes32 indexed l1Asset, address indexed l2Asset, uint256 amount);
55    event DepositFailure(address indexed receiver, bytes32 indexed l1Asset, address indexed l2Asset, uint256 amount);
56
57    constructor(address mailbox) MailboxClient(mailbox) {}
58
59    function initialize(address owner, address hook, address ism, ExtAddr memory ccdmHost) public initializer {
60        _MailboxClient_initialize(hook, ism, owner);
61
62        StorageV1 storage $ = _getStorageV1();
63
64        $._ccdmHost = ccdmHost;
65    }

Recommendation:

We advise a constructor to be introduced that either invokes the initializer modifier of the Initializable contract or invokes the Initializable::_disableInitializers function to prevent the base implementation from ever being initialized.

Alleviation (58e8cc66dfa900c03c47df78f5170d9960005629):

An Initializable::initialize modifier has been introduced to the contract's CCDMClient::constructor thereby preventing re-initializations as long as the contract does not utilize a versioned initialization system.

If such a system is expected, we advise the Initializable::_disableInitializers function instead.

CCM-02M: Improper Refund Fee Acquisition

Type	Severity	Location
Logical Fault		CCDMClient.sol:L109, L140, L145, L156-L163

Description:

In contrast to CCDMClient::adjust operations, any cross-chain message handled that requires a refund will have its operation funded by the CCDMClient (i.e. MailboxClient), which is incorrect as the contract may be insufficiently funded and should not incur operational costs itself.

Additionally, CCDMClient::handle operations are payable yet the difference between the msg.value and the actual native funds utilized is not tracked in contrast to CCDMClient::adjust.

Impact:

Native funds are improperly handled by the CCDMClient::handle with varying effects including failure of transactions that should otherwise succeed as well as lock of funds to the contract.

Example:

Copy
125function _handle(uint32, bytes32, bytes calldata rawMsg) internal {
126    MsgDeposit memory msg_ = Message.decodeDeposit(rawMsg);
127
128    StorageV1 storage $ = _getStorageV1();
129
130    VaultInfo storage info = $._vaults[$._vaultIdxByL1Asset[msg_.token]];
131
132    ISudoVault vault = info.vault;
133
134    if (address(vault) == address(0x0)) {
135        revert Error.InvalidDepositRequest("non-registered asset");
136    }
137
138    try vault.manualDeposit(msg_.amount, msg_.receiver.toAddress()) returns (uint256 spent) {
139        if (spent < msg_.amount) {
140            _processRefund(msg_.receiver, msg_.token, msg_.amount - spent);
141        }
142        _processSpent(msg_.receiver, msg_.token, spent);
143        emit DepositSuccess(msg_.receiver.toAddress(), info.l1Asset, info.l2Asset, spent);
144    } catch {
145        _processRefund(msg_.receiver, msg_.token, msg_.amount);
146        emit DepositFailure(msg_.receiver.toAddress(), info.l1Asset, info.l2Asset, msg_.amount);
147    }
148}

Recommendation:

We advise these issues to be resolved by ensuring that native funds are adequately handled by the CCDMClient::handle function and its inner calls, validating that the caller has supplied sufficient funds to process the refunds and refunding them with any remainder in case a refund was not processed.

Alleviation (5297bb74fa5cb1c63239172a7a7a3a7c8ce808e3):

A new ATM contract has been introduced throughout the system that is responsible for funding the system with native funds when fees are necessary. This contract has been correctly integrated by the CCDMClient::_processRefund function, addressing this exhibit.

CCM-03M: Unhandled Failure of Cross-Chain Message

Type	Severity	Location
Logical Fault		CCDMClient.sol:L130, L134-L136

Description:

The CCDMClient::_handle function will result in a revert error if a vault has not been registered for the asset being bridged. In turn, those assets will not be encoded in a refund payload to the origin chain causing them to be lost to the contract.

Impact:

Any unsupported bridged assets to the CCDMClient will be blocked until a vault has been deployed for them.

Example:

Copy
125function _handle(uint32, bytes32, bytes calldata rawMsg) internal {
126    MsgDeposit memory msg_ = Message.decodeDeposit(rawMsg);
127
128    StorageV1 storage $ = _getStorageV1();
129
130    VaultInfo storage info = $._vaults[$._vaultIdxByL1Asset[msg_.token]];
131
132    ISudoVault vault = info.vault;
133
134    if (address(vault) == address(0x0)) {
135        revert Error.InvalidDepositRequest("non-registered asset");
136    }
137
138    try vault.manualDeposit(msg_.amount, msg_.receiver.toAddress()) returns (uint256 spent) {
139        if (spent < msg_.amount) {
140            _processRefund(msg_.receiver, msg_.token, msg_.amount - spent);
141        }
142        _processSpent(msg_.receiver, msg_.token, spent);
143        emit DepositSuccess(msg_.receiver.toAddress(), info.l1Asset, info.l2Asset, spent);
144    } catch {
145        _processRefund(msg_.receiver, msg_.token, msg_.amount);
146        emit DepositFailure(msg_.receiver.toAddress(), info.l1Asset, info.l2Asset, msg_.amount);
147    }
148}

Recommendation:

We advise the code to instead refund the assets if no vault exists to the base chain, ensuring that assets cannot be lost if they are pre-emptively bridged.

Alleviation (d94d2a63d25db5623d69dc33aea6e4fdd011d669):

The code was updated at the CCDMHost contract instead, disallowing the relay of tokens not explicitly supported in the system. While it is an adequate alleviation, we still advise the error to be handled correctly at the CCDMClient implementation instead as other unforeseen errors may occur in future upgrades of the system and those should be gracefully handled.