Description

The oracle contract has the updateMptHash and updateFpHash external functions for submitting oracle proofs using the traditional MPT and ZK verifiers, respectively:

function updateMptHash(uint16 _sourceChainId, bytes32 _blockHash, bytes32 _receiptHash, address _userApplication) external {
    _updateHash(_sourceChainId, _blockHash, _receiptHash, _blockHash, _receiptHash, _userApplication);
}

function updateFpHash(uint16 _sourceChainId, bytes32 _blockHash, bytes calldata zkMptProof, address _userApplication) external {
    require(address(zkMptValidator)!=address(0),"ZkBridgeOracle:Not set zkMptValidator");
    IZKMptValidator.Receipt memory receipt = zkMptValidator.validateMPT(zkMptProof);
    _updateHash(_sourceChainId, _blockHash, receipt.receiptHash, receipt.logsHash, receipt.logsHash, _userApplication);
}

Either function can be called to submit a proof since both are external functions. The functions should generally be separated as each oracle is supposed to perform a single form of verification.

Impact

If a User Agent (UA) owner configured the Ultra Light Node (ULN) to set the oracle address to that of the deployed ZkBridgeOracle contract with the intent of using the ZK verifier, it would also be possible to submit an MPT proof—defeating the owner's intent.

This would let the attacker submit hashes using the traditional MPT verifier despite the owner expecting the ZK verifier to be used.

Recommendations

Consider separating the two verifier proof-submitting functions into separate contracts so that a ZK oracle cannot be used to submit an MPT proof and an MPT oracle cannot have a ZK proof be submitted to it.

Remediation

Polyhedra provided the following response to this finding:

In our design context, both MPT and FP methods go through ZK verification. The function updateMptHash is utilized to upload blockHash and receiptHash to ULN and validates whether blockHash and receiptHash exist in our BlockUpdater. During the block update, ZK verification is involved in the BlockUpdater contract.

On the other hand, updateFpHash is used to upload transaction logs to ULN. Initially, it goes through a ZkMPT check, then verifies the existence of the block, and eventually pushes the logs to ULN.

These two functions are designed to submit different content, and one cannot be used to submit the data of the other. Given these system behaviors, we respectfully disagree with the recommendation as we don't see the need to separate these functions into different contracts in our case.

After an internal discussion, we agree that a relayer using the MPT proof library should only be interacted with via the updateMptHash or batchUpdateMptHash functions, and the FP proof library via the updateFpHash and batchUpdateFpHash functions.

However, we are still concerned about the potential possibility of type confusion attacks between the receiptHash and blockHash submitted by the functions. For example, if the keccak256 of an FP-submitted packet happens to match a block hash, the relayer could change the validation library to MPTValidator01 and bypass oracle safety checks.

It is also possible to set the confirmation count on a lookupHash and blockHash that should be submitted through another function. Though doing this has no impact, the oracle's process for updating hashes should match the validation library for correctness reasons.

Additionally, LayerZero may release validation libraries in the future independently of Polyhedra that unintentionally enable type confusion attacks.

In general, it makes sense that one oracle has one validation method. Separating validation methods reduces the attack surface and mitigates an entire class of potential attacks.