PolygonZkEvmBridgeAdapter Manual Review Findings

PZE-01M: Inexistent Initialization Protection of Base Implementation

Type	Severity	Location
Language Specific		PolygonZkEvmBridgeAdapter.sol:L31

Description:

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

Example:

Copy
30contract PolygonZkEvmBridgeAdapter is IBridgeAdapter, OwnableUpgradeable, PolygonZkEvmBridgeAdapterStorageV1 {
31    function initialize(IPolygonZkEVMBridge bridge_, uint32 destNetwork_, bool forceUpdateGlobalExitRoot_)
32        public
33        initializer
34    {
35        __Ownable_init();
36
37        _setBridge(bridge_);
38        _setDestNetwork(destNetwork_);
39        _setForceUpdateGlobalExitRoot(forceUpdateGlobalExitRoot_);
40    }

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):

A PolygonZkEvmBridgeAdapter::constructor has been introduced invoking the Initializable::initialize modifier 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.

PZE-02M: Inexistent Claim of Bridged Assets

Type	Severity	Location
Language Specific		PolygonZkEvmBridgeAdapter.sol:L23-L25

Description:

In contrast to the Optimism and Arbitrum L1-to-L2 bridges, the Polygon zkEVM bridge requires the recipient of the bridged assets to manually claim them.

Impact:

Presently, any assets that are bridged will require manual intervention to go through as they are not automatically processed.

Example:

Copy
23function bridgeAsset(address destAddr, address l1Asset, address, uint256 amount) external {
24    bridge.bridgeAsset(destNetwork, destAddr, amount, l1Asset, forceUpdateGlobalExitRoot, "");
25}

Recommendation:

We advise such a flow to be introduced to the BasicVault implementation, ensuring that all bridged assets are properly claimed at their destination chain.

Alleviation (d94d2a63d25db5623d69dc33aea6e4fdd011d669):

The Mitosis team evaluated this exhibit and has opted to acknowledge it as in the Polygon zkEVM mainnet, the Zenland deployer executes every claim per the Mitosis team's investigation.

Additionally, the Mitosis team stated that they will monitor the situation and execute their own bot to ensure their transactions are claimed, thus acknowledging this exhibit.

PZE-03M: Inexistent Approval of Bridged Assets

Type	Severity	Location
Logical Fault		PolygonZkEvmBridgeAdapter.sol:L24

Description:

The PolygonZkEvmBridgeAdapter::bridgeAsset function will not approve the necessary l1Asset amount to the bridge, causing bridge operations to fail.

Impact:

Bridge operations are presently impossible as an approval has not been authorized to the bridge for it to escrow the funds bridged.

Example:

Copy
23function bridgeAsset(address destAddr, address l1Asset, address, uint256 amount) external {
24    bridge.bridgeAsset(destNetwork, destAddr, amount, l1Asset, forceUpdateGlobalExitRoot, "");
25}

Recommendation:

We advise an approval operation to be performed beforehand, preferably in a secure way such as by using the SafeERC20 library by OpenZeppelin and specifically the SafeERC20::forceApprove function.

Alleviation (d94d2a63d2):

An approval is correctly performed in the latest implementation, however, it is utilizing the SafeERC20::safeIncreaseAllowance method instead of the forced approval mechanism we advised. As such, we consider this exhibit partially alleviated.

Alleviation (58e8cc66df):

The code was updated to utilize the SafeERC20::forceApprove function thus ensuring that the approval is forced to the new value and is not incremented.

As such, we consider this exhibit fully alleviated.