July 18, 2023
This security assessment was prepared by OpenZeppelin.
We audited the mantle-token-contracts repository at the b2016dfb932d85b8b33a9294e8280aa04ca46975 commit and the mantle repository at the d627d242fe19f50f344f1ff4b27532d1757303a6 commit.
In scope were the following contracts:
contracts
├── L1
│ └── L1MantleToken.sol
└── Migration
└── MantleTokenMigrator.sol
packages
└── contracts
└── contracts
├── L1
│ └── messaging
│ └── L1StandardBridge.sol
└── L2
└── messaging
└── L2StandardBridge.sol
When reviewing the bridge contracts, we assumed the cross-domain messengers work as documented.
The Mantle token (MNT) is the gas token of the Mantle network. On the Ethereum network, it is an upgradeable ERC-20 token with mint, burn, permit, and vote extensions. In the default configuration, it allows minting to occur at most once a year and has a configurable mint cap that limits how many tokens can be minted at once, though both parameters can be changed by the owner.
The migrator contract facilitates migration from BIT to MNT by holding MNT tokens and exchanging them for BIT at a specified ratio. It also has the functionality to recover mistakenly sent tokens as well as send BIT and MNT tokens from the contract to a treasury address.
The bridge is a fork of the Optimism bridge contracts, extended to handle the Mantle token. It allows anyone to lock ETH or arbitrary ERC-20 tokens on the Ethereum mainnet in exchange for corresponding ERC-20 tokens on the Mantle network, which can later be destroyed to recover the original tokens. It is worth noting that the bridge itself does not guarantee any properties of the layer 2 tokens, or any correspondence with their layer 1 (L1) counterparts. For instance, any non-standard features of the original token such as rebasing, blacklists or pause functionality will not be replicated on the layer 2 (L2) contract unless it is specifically designed that way.
The Mantle token is upgradeable, meaning that the mint cooldown and other constants can be changed as well as the behavior of the token.
The bridge attempts to identify valid token exchanges, but with the exception of ETH and MNT, it trusts the L2 tokens to be able to identify the corresponding L1 token address. Therefore, the onus is on users to validate that they only interact with trustworthy tokens.
The owner of the Mantle token can upgrade the contract, mint new tokens, change the mint cap, and transfer the ownership.
The owner of the migrator contract can pause and unpause the contract, change the treasury address, transfer BIT and MNT tokens to the treasury address, and transfer all the other ERC-20 tokens to an arbitrary address.
The Mantle team has claimed that both contracts will be owned by the governance.
The following misleading comments were identified:
setMintCapNumerator references a MintCapNumeratorSet event, but it should be MintCapNumeratorChanged.mint function says the mint time interval "is initially set to 1 year", which suggests it could be updated. It is actually a constant and can only be changed if the whole contract is upgraded.Consider updating the comments accordingly.
Update: Resolved in pull request #50 at commit 2a04393.
The L1StandardBridge contains a guard condition to prevent it from being reinitialized. However, it assumes the messenger will be non-zero after initialization, which is not guaranteed by the initialize function. This means it is possible to initialize the other variables and later overwrite them. In practice, the contract will be non-functional until the messenger is set.
Nevertheless, in the interest of predictability, consider ensuring the messenger is non-zero during initialization.
Update: Resolved in pull request #1027 at commit e641f0e.
The L1StandardBridge implementation contract sets the messenger to the zero address, but this doesn't prevent it from being initialized.
In the interest of limiting the attack surface, consider ensuring the implementation contract cannot be initialized. This could be achieved by setting the messenger to an unused non-zero address.
Update: Acknowledged, not resolved. The Mantle team stated:
We will use our proxy contract to initialize right after deploying the bridge contract, and the proxy contract will be required to be initialized only once. Even if someone tries to initialize the implementation contract afterwards, it will have no impact on our proxy contract.
In MantleTokenMigrator.sol the received word is misspelled as recieved in several places. Consider resolving this typographical error.
Update: Resolved in pull request #53 at commit 6b78f54. There have been unrelated changes that removed several instances.
The MantleTokenMigrator contract doesn't expect to receive ETH and explicitly reverts on the receive and fallback functions. As noted in the Solidity documentation, without the receive, fallback, and payable functions a Solidity contract reverts on receiving ETH by default. Consider removing the receive and fallback functions to simplify the codebase.
Update: Resolved in pull request #45 at commit c662f74.
The setMintCapNumerator and setTreasury functions can consume less gas by emitting an event first and then changing the storage variable.
For example, the following code snippet
uint256 previousMintCapNumerator = mintCapNumerator;
mintCapNumerator = _mintCapNumerator;
emit MintCapNumeratorChanged(msg.sender, previousMintCapNumerator, mintCapNumerator);
may be rewritten as:
emit MintCapNumeratorChanged(msg.sender, mintCapNumerator, _mintCapNumerator);
mintCapNumerator = _mintCapNumerator;
Consider rewriting the setMintCapNumerator and setTreasury functions to save gas.
Update: Resolved in pull request #48 at commit 4159ef3.
Some imprecise docstrings have been identified:
setMintCapNumerator function does not follow the Ethereum Natural Specification Format (NatSpec) format.migrateAllBIT description, the migrateBIT description should note that the _amount must be non-zero.Consider updating the docstrings accordingly.
Update: Resolved in pull request #54 at commits 97d88d6 and 0e76225, and pull request #55 at commit d515706.
The sweepTokens function of the MantleTokenMigrator contract redundantly casts both known token addresses to the address type. Consider removing the unnecessary cast operations.
Update: Resolved in pull request #44 at commit bb921bf.
The L1MantleToken contract inherits from several contracts with redundant dependencies. This means that some of the contracts are inherited both directly and indirectly. For example, inheriting ERC20VotesUpgradeable makes inheriting ERC20PermitUpgradeable and ERC20Upgradeable redundant.
This is still a reasonable pattern because it improves explicitness and makes the sequence of initializations more intuitive. However, it also forces the L1MantleToken to introduce boilerplate functions that are unrelated to the token's new logic. Our opinion is that removing redundancy from the inheritance chain would make the contract simpler and easier to reason about. Consider limiting the inheritance chain to the necessary contracts (i.e., ERC20BurnableUpgradeable, OwnableUpgradeable and ERC20VotesUpgradeable).
Update: Acknowledged, not resolved. The Mantle team stated:
We prefer not to make modifications to improve explicitness and make the initialization sequence more intuitive.
The bridge contracts have specific logic to handle MNT tokens but the L1StandardBridge contract currently associates the BIT token with the L2 Mantle token. Consider reusing the existing variable to identify the MNT token address.
Note that the L2 token also still references the BIT token and should be updated accordingly.
Update: Resolved in pull request #1075 at commit d140d18. The correct address is now hardcoded instead of reusing the variable.
The L1StandardBridge has a function to donate ETH to the contract. However, this function is a holdover from the Optimism code base and is not required for a fresh deployment. Consider removing it.
Update: Resolved in pull request #1029 at commit a47a661.
All deposits and withdrawals pass an arbitrary data parameter over the bridge. This parameter is emitted in the events on both sides, but is otherwise unused. The documentation claims it is a convenience for external contracts, but it is not passed to the destination and other contracts cannot read the events. Consider clarifying how the parameter could be used, or remove it from the transfer.
Update: Acknowledged, not resolved. The Mantle team stated:
We see this as a data interface reserved for subsequent cross-chain interoperability.
The bridge contracts contain custom logic to support depositing Mantle tokens. However, only the token mapping validation differs from the generic ERC-20 case, so the finalizeDeposit calldata specification can be unified between both branches. Similarly, as long as the BVM_MANTLE token is configured correctly Mantle tokens can be withdrawn using the generic ERC-20 logic. This would remove unnecessary withdrawal logic and make the finalizeMantleWithdrawal function obsolete. Consider simplifying the code accordingly.
Update: Acknowledged, not resolved. The Mantle team stated:
We divided this method mainly to facilitate the addition of necessary restrictions for the L2 native token, and also to facilitate subsequent targeted maintenance.
The finalizeMantleWithdrawal function validates that it is called from the L2 bridge, but this check is repeated on the finalizeERC20Withdrawal function. Consider removing the redundant validation.
Update: Resolved in pull request #1031 at commit 5fbb898.
The _initiateWithdrawal function claims to retrieve tokens from the _from address but it actually takes them from the caller. Similarly, the event references the caller instead of the _from address. This behavior is equivalent in the current code base because both calling functions pass the message sender as the _from address. Nevertheless, in the interest of predictability and encapsulation, consider using the _from address inside the function.
Update: Acknowledged, not resolved. The Mantle team stated:
We will consider this in future Mantle Network upgrades.
The L1StandardBridge contract has the address of the MNT token hardcoded. Consider creating a constant variable and using it instead of the hardcoded address for clarity and readability.
Similarly, the L2StandardBridge contract hardcodes the IL2StandardERC20 identifier. Consider using the more expressive type(IL2StandardERC20).interfaceId statement instead.
Update: Acknowledged, not resolved. The Mantle team stated:
We will consider this in future Mantle Network upgrades. For now, we have decided to hardcode this address as we do not foresee the MNT token address changing in the future as the MNT token itself is upgradable.
Several minor vulnerabilities and notes have been found throughout the codebase, and fixes have been suggested. We found the codebase to be well documented and appreciate the reuse of existing libraries and systems.
Below we provide our recommendations for monitoring important activities in order to detect and prevent potential bugs.
While audits help in identifying code-level issues in the current implementation and potentially the code deployed in production, the Mantle team is encouraged to consider incorporating monitoring activities in the production environment. Ongoing monitoring of deployed contracts helps identify potential threats and issues affecting production environments. With the goal of providing a complete security assessment, the monitoring recommendations section raises several actions addressing trust assumptions and out-of-scope components that can benefit from on-chain monitoring.
There are multiple privileged actions with serious security implications:
L1MantleToken contract owner can :
MantleTokenMigrator contract owner can:
L1StandardBridge contract owner can upgrade the contract, which gives the owner access to all funds secured by the bridge.Consider monitoring these administrator functions to ensure all changes are expected.
Consider monitoring the exchanges that occur on the MantleTokenMigrator contract to ensure they are within reasonable bounds. Unusually large, small or frequent transactions may indicate an unexpected edge case.
Consider monitoring the token transfers over the bridge to identify:
These may indicate a user interface bug, an ongoing attack or other unexpected edge cases.