CCDMHost Manual Review Findings

CCD-01M: Inexistent Initialization Protection of Base Implementation

Type	Severity	Location
Language Specific		CCDMHost.sol:L49

Description:

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

Example:

Copy
44contract CCDMHost is IMessageRecipient, Router, CCDMHostStrorageV1 {
45    using Conv for address;
46    using Conv for bytes32;
47    using EnumerableMapExtended for EnumerableMapExtended.UintToBytes32Map;
48
49    constructor(address mailbox) Router(mailbox) {}
50
51    function initialize(address owner, address hook, address ism) public initializer {
52        _MailboxClient_initialize(hook, ism, owner);
53    }

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 CCDMHost::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.

CCD-02M: Improper Refund Processing

Type	Severity	Location
Language Specific		CCDMHost.sol:L133-L139

Description:

The CCDMHost and CCDMClient contracts will improperly handle refund operations in their cross chain messages as they assume the same address is owned by the same party across chains; a trait that is not guaranteed especially when the addresses are smart contracts and not EOAs.

Impact:

Any cross-chain transaction failures may result in refunds being performed to addresses that are not in ownership of the original transaction initiator, resulting in potential loss of funds.

Example:

Copy
125/// @dev must receives MsgRefund, MsgBridge
126function _handle(uint32 origin, bytes32 sender, bytes calldata rawMsg) internal override {
127    MsgType msgType = Message.msgType(rawMsg);
128
129    if (msgType == MsgType.Deposit) {
130        revert Error.InvalidMsgType(uint8(msgType));
131    }
132
133    if (msgType == MsgType.Refund) {
134        MsgRefund memory msgRefund = Message.decodeRefund(rawMsg);
135
136        SafeERC20.safeTransfer(
137            IERC20(msgRefund.token.toAddress()), msgRefund.receiver.toAddress(), msgRefund.amount
138        );
139    } else if (msgType == MsgType.Bridge) {
140        MsgBridge memory msgBridge = Message.decodeBridge(rawMsg);
141
142        StorageV1 storage $ = _getStorageV1();
143
144        address l1Asset = msgBridge.token.toAddress();
145        address l2Asset = $._assetL1L2Map[origin][l1Asset];
146
147        SafeERC20.safeIncreaseAllowance(IERC20(l1Asset), address(this), msgBridge.amount);
148
149        $._bridges[origin].bridgeAsset(sender.toAddress(), l1Asset, l2Asset, msgBridge.amount);
150    }
151}

Recommendation:

We advise all cross-chain messages to permit explicitly-specified refund addresses per layer, ensuring that the caller is responsible for specifying an address they have ownership of on each chain.

Alleviation (5297bb74fa5cb1c63239172a7a7a3a7c8ce808e3):

A new entry to the cross-chain messages has been introduced that denotes which address refunds should occur to, addressing this exhibit in full.

CCD-03M: Incorrect Validation of Native Funds

Type	Severity	Location
Mathematical Operations		CCDMHost.sol:L82, L86

Description:

The CCDMHost::deposit function will incorrectly validate the incoming msg.value attached to the call, permitting it to be lower than the hplFee and userFee combined which would result in an underflow error in the ensuing line.

Impact:

A minor severity has been assigned to this exhibit as the code would still be operable if the msg.value is supplied exactly and would simply revert in all other circumstances.

Example:

Copy
82if (hplFee + userFee < msg.value) {
83    revert Error.InsufficientFee();
84}
85
86uint256 refund = msg.value - hplFee - userFee;

Recommendation:

We advise the referenced conditional to evaluate whether the msg.value is less than those values combined instead, ensuring that the code correctly reverts with a descriptive error message in such a case.

Alleviation (d94d2a63d25db5623d69dc33aea6e4fdd011d669):

The conditional has been adjusted as advised, evaluating whether the msg.value is less than the expected fees and reverting in such a case as is the expected behaviour of the code.

CCD-04M: Incorrect Approval Operation

Type	Severity	Location
Logical Fault		CCDMHost.sol:L147

Description:

The CCDMHost::_handle function will permit an approval to self when the incoming message type is a Bridge message which is invalid.

Impact:

It is presently impossible for bridge operations to be performed as the CCDMHost contract will improperly approve itself rather than transfer the funds to the bridge for further processing.

Example:

Copy
147SafeERC20.safeIncreaseAllowance(IERC20(l1Asset), address(this), msgBridge.amount);
148
149$._bridges[origin].bridgeAsset(sender.toAddress(), l1Asset, l2Asset, msgBridge.amount);

Recommendation:

We advise the contract to transfer the assets directly to the $._bridges[origin] destination before invoking the IBridgeAdapter::bridgeAsset function, ensuring that the bridge can properly fund the cross-chain operation.

Alleviation (58e8cc66dfa900c03c47df78f5170d9960005629):

The code was updated to properly perform an approval to the target bridge which will be captured by it, thereby alleviating this exhibit.