OmnichainFungibleFractionsDO Manual Review Findings

OFF-01M: Incorrect Encoding of Callback Payloads

Type	Severity	Location
Logical Fault		OmnichainFungibleFractionsDO.sol: • I-1: L328 • I-2: L389

Description:

The IOmnichainFungibleFractionsOperations::omnichainIncreaseBalanceCallback function selector as an operation will result in the OmnichainNonFungibleTokenDO::_omnichainCallback function being invoked which will attempt to decode a bool from the opData.

As the data supplied is empty rather than true, the decoding operation will fail causing a cross-chain LayerZero transaction that cannot be executed to manifest.

Impact:

Repeated omni-chain transfer attempts that were executed before the transaction was successfully processed will result in hanged cross-chain callback transactions.

Example:

Copy
318function _omnichainIncreaseBalance(
319    DataPoint dp,
320    bytes32 rid,
321    OmnichainAddress from,
322    OmnichainAddress to,
323    uint256 id,
324    uint256 value
325) internal returns (bytes memory) {
326    if (_successfullOmnichainRequests[rid]) {
327        // Return data for callback call without any actions
328        return abi.encode(dp, IOmnichainFungibleFractionsOperations.omnichainIncreaseBalanceCallback.selector, "");
329    }
330    _successfullOmnichainRequests[rid] = true;
331
332    // Now we can actually increase balance
333    (, address toAddress) = OmnichainAddresses.decode(to);
334    bytes32 diidTarget = _diid(dp, toAddress);
335    DiidData storage diiddTarget = _diidData(dp, diidTarget);
336    return _updateOmnichainBalancesOnTransfer(dp, from, toAddress, id, value, diidTarget, diiddTarget);
337}

Recommendation:

We advise the code to properly encode the true status code so as to ensure that the callback is properly executed and acts as a "no-op" in case an omni-chain transfer was retried multiple times before it was successfully executed.

Alleviation (c6b23c23d8bcd8cce85049ad959cbd711a37126b):

The overall logic around omni-chain request statuses was updated to incorporate a CallResult value and the callback payload in both referenced statements will ensure that a bool value is encoded rather than an empty one, addressing this exhibit in full.

OFF-02M: Breach of EIP-1155 Standard

Type	Severity	Location
Standard Conformity		OmnichainFungibleFractionsDO.sol:L617-L628

Description:

Per the EIP-1155 standard, a mint operation is considered a specialized transfer operation that must still adhere to the EIP-1155 callback mechanism.

A refund of an omni-chain transfer will not trigger this callback operation thereby breaching the standard.

Impact:

The EIP-1155 standard is presently not adhered to by the failed omni-chain transfer refund mechanism as it mints EIP-1155 assets without informing the recipient.

Example:

Copy
617function _refundOmnichainTransfer(DataPoint dp, bytes32 rid) private {
618    PendingOmnichainTransfer memory pot = _pendingOmnichainTransfers[rid];
619    (, address fromAddress) = OmnichainAddresses.decode(pot.from);
620    bytes32 diidFrom = _diid(dp, fromAddress);
621
622    DiidData storage diiddFrom = _diidData(dp, diidFrom);
623    uint256 length = pot.ids.length;
624    for (uint256 i; i < length; i++) {
625        _increaseBalance(diiddFrom, pot.ids[i], pot.values[i], dp, diidFrom);
626    }
627    emit OmnichainTransferRefund(rid, dp, fromAddress, pot.ids, pot.values);
628}

Recommendation:

We advise the standard to be adhered to as many smart contracts explicitly rely on EIP-1155 callback hooks to track such transfers.

To note, special precautions would need to be taken to ensure a re-entrancy cannot occur such as deleting the _pendingOmnichainTransfers mapping prior to performing the refund.

Alleviation (c6b23c23d8):

The code was refactored to perform a chain of operations that ultimately result in the safe acceptance check being invoked after the balance changes have taken effect, alleviating the original exhibit.

Our recommendation to delete the _pendingOmnichainTransfers data entry prior to informing the recipient of the transfer was not heeded, permitting re-entrancy attacks to manifest via the relevant callback.

We advise the relevant order of operations to be updated, preventing this misbehavior from manifesting.

Alleviation (18bd9a145a):

The code was updated per our recommendation, ensuring that the _pendingOmnichainTransfers data entry is deleted prior to processing the refund request and thus alleviating this exhibit in full.

OFF-03M: Improper Refund Address

Type	Severity	Location
Logical Fault		OmnichainFungibleFractionsDO.sol:L518

Description:

The referenced OmnichainProxy function invocation will assume that the caller themselves own the same address on the target chain which is incorrect as many L2s employ address masking, especially in the context of smart contract refund recipients (i.e. multi-signature wallets).

Impact:

A cross-chain message's surplus or failure may result in unspent native funds being erroneously sent to an incorrect / inaccessible refund recipient, resulting in minor fund loss.

Example:

Copy
493function _retryIncreaseOmnichainBalance(bytes32 rid) internal nonReentrant {
494    PendingOmnichainTransfer memory pot = _pendingOmnichainTransfers[rid];
495    (uint32 toChainId, ) = OmnichainAddresses.decode(pot.to);
496
497    bytes memory data;
498    bytes4 operation;
499
500    if (pot.ids.length > 1) {
501        data = abi.encode(rid, pot.from, pot.to, pot.ids, pot.values);
502
503        operation = IOmnichainFungibleFractionsOperations.omnichainBatchIncreaseBalance.selector;
504    } else {
505        data = abi.encode(rid, pot.from, pot.to, pot.ids[0], pot.values[0]);
506
507        operation = IOmnichainFungibleFractionsOperations.omnichainIncreaseBalance.selector;
508    }
509
510    bytes32 retryRid = _proxy.queryDataObjectWrite{value: msg.value}(
511        toChainId,
512        address(this), // Address of DO on target chain is same on all chains
513        pot.dp,
514        operation,
515        data,
516        OMNICHAIN_INCREASE_BALANCE_GAS_LIMIT,
517        OMNICHAIN_INCREASE_BALANCE_CALLBACK_GAS_LIMIT,
518        payable(_msgSender())
519    );
520    emit OmnichainRetryIncompleteOmnichainIncreaseBalanceSent(rid, retryRid);
521}

Recommendation:

We advise the system to permit a different refund address to be specified, ensuring that message refund operations are executed correctly.

Alleviation (c6b23c23d8bcd8cce85049ad959cbd711a37126b):

The refund address was updated to be the result of an input argument, alleviating this exhibit.