ArbitrumBridgeAdapter Manual Review Findings

ABA-01M: Inexistent Initialization Protection of Base Implementation

Type	Severity	Location
Language Specific		ArbitrumBridgeAdapter.sol:L36, L37

Description:

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

Example:

Copy
36contract ArbitrumBridgeAdapter is IBridgeAdapter, OwnableUpgradeable, ArbitrumBridgeAdapterStorageV1 {
37    function initialize(IArbitrumGatewayRouter bridge_, IATM atm_, uint32 maxGas_, uint32 gasPriceBid_)
38        public
39        initializer
40    {
41        __Ownable_init();
42
43        _setATM(atm_);
44        _setGas(maxGas_, gasPriceBid_);
45        _setBridge(bridge_);
46    }

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

ABA-02M: Inexistent Approval of Bridged Assets

Type	Severity	Location
Logical Fault		ArbitrumBridgeAdapter.sol:L34

Description:

The ArbitrumBridgeAdapter::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
33function bridgeAsset(address destAddr, address l1Asset, address, uint256 amount) external {
34    bridge.outboundTransfer(l1Asset, destAddr, amount, maxGas, gasPriceBid, "");
35}

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.