Description

Messaging libraries are responsible for notifying DVNs of messages being sent ("send library") on the source chain and receiving verifications from DVNs ("receive library") on the destination chain.

They verify the payload of each packet, committing the verified payload hash to the endpoint after the extrinsic security requirement (e.g., DVN threshold) is fulfilled.

Any send library in ZkBridgeOracleV2's messageLibLookup with enabled set to true may call the assignJob function. If the message's dstEid is the localEid, the DVN skips the off-chain component and shortcuts to verify with the legitimate receive library:

function assignJob(AssignJobParam calldata _param, bytes calldata /*_options*/ )
external
payable
returns (uint256 fee)
{
    if (!supportedDstChain[_param.dstEid]) revert UnsupportedChain(_param.dstEid);
    if (!isSupportedMessageLib(msg.sender)) revert UnsupportedSendLib();
    fee = chainFeeLookup[_param.dstEid];
    emit OracleNotified(_param.dstEid, _param.confirmations, _param.sender, fee);

!    if (localEid == _param.dstEid) {
!        (address receiverLib,) = layerZeroEndpointV2.getReceiveLibrary(_param.packetHeader.receiver(), localEid);
!        IReceiveUlnE2(receiverLib).verify(_param.packetHeader, _param.payloadHash, _param.confirmations);
!    }
}

If a malicious library were to be added (e.g., if an upgradable library were to be added whose owner's keys were compromised), it is possible for the library to prevent itself from being unconfigured from the DVN because the removal function makes external calls to the malicious library where it could revert:

function removeLzMessageLib(address _messageLib) external onlyOwner {
    if (!messageLibLookup[_messageLib].enabled) revert MessageLibAlreadyDeleted();

    messageLibLookup[_messageLib].enabled = false;
!    uint256 fee = ISendLib(_messageLib).fees(address(this));
    if (fee > 0) {
!        ISendLib(_messageLib).withdrawFee(payable(owner()), fee);
        emit WithdrawFee(_messageLib, owner(), fee);
    }
}

Impact

A malicious library can prevent itself from being removed, which would permanently impact the integrity of the DVN.

Recommendations

Ensuring legitimacy of libraries

First, to reduce centralization risk by ensuring both Polyhedra Network and LayerZero Labs agree on the legitimacy and security of a messaging library, we recommend querying the LayerZero endpoint to ensure the _messageLib being added is a registered library:

function addLzMessageLib(address _messageLib) external onlyOwner {
+   require(
+       layerZeroEndpointV2.isRegisteredLibrary(_messageLib), 
+       "message library not registered with LayerZero"
+   );
    // ...
}

Ensuring malicious libraries can be removed

Additionally, to ensure malicious send libraries can be removed, we recommend removing the fee-withdrawal logic from the removeLzMessageLib function; a function to withdraw fees already exists. It is named withdrawFee. So, having the logic in removeLzMessageLib only adds risk.

Alternatively, consider adapting the removeLzMessageLib to use a try-catch such that even in the case that any of the _messageLib functions were to revert, the messageLibLookup entry would still be removable.

Remediation

Polyhedra Network provided the following response to this finding:

MessageLib is added via our multi-signature wallet in this function. During the addition process, we conduct thorough verification and review of the contract source code to ensure that the messageLib meets our requirements. Subsequently, we will upgrade the code to add support for v1 301. The Endpoint in v1 does not have this function for verification and is not compatible with the v2 Endpoint. Therefore, the audit and verification of adding messageLib should occur off-chain.