OptimismBridgeAdapter Manual Review Findings

OBA-01M: Inexistent Initialization Protection of Base Implementation

Type	Severity	Location
Language Specific		OptimismBridgeAdapter.sol:L30

Description:

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

Example:

Copy
29contract OptimismBridgeAdapter is IBridgeAdapter, OwnableUpgradeable, OptimismBridgeAdapterStorageV1 {
30    function initialize(IOptimismGateway bridge_, uint32 minGasLimit_) public initializer {
31        __Ownable_init();
32
33        _setGas(minGasLimit_);
34        _setBridge(bridge_);
35    }

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

OBA-02M: Inexistent Approval of Bridged Assets

Type	Severity	Location
Logical Fault		OptimismBridgeAdapter.sol:L20

Description:

The OptimismBridgeAdapter::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
19function bridgeAsset(address destAddr, address l1Asset, address l2Asset, uint256 amount) external {
20    bridge.depositERC20To(l1Asset, l2Asset, destAddr, amount, minGasLimit, "");
21}

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 continues to use the SafeERC20::safeIncreaseAllowance function instead of the SafeERC20::forceApprove function in contradiction to the ArbitrumBridgeAdapter and PolygonZkEvmBridgeAdapter implementations.

As such, we continue to consider this exhibit partially alleviated.

Alleviation (0d29218ad3):

The code was updated to utilize the SafeERC20::forceApprove function over the SafeERC20::safeIncreaseAllowance variant, rendering this exhibit fully alleviated.