Mitosis audit
A private security review of Mitosis (cross-chain liquidity layer 1), conducted with Three Sigma and completed on 25th June 2024 over 11 days. 0xSimao disclosed 32 findings: 1 high severity, 6 medium, 6 low and 19 informational.
What Mitosis is
Mitosis is an ecosystem-owned-liquidity layer 1: LPs deposit assets on connected chains and receive miAssets on the hub, governance allocates that pooled liquidity across networks and protocols, and Hyperlane-style mailboxes carry the messages between them.
Scope
The review covered the deposit and redemption pipeline: vaults on spoke chains, the redeem queue on the hub that matches exits to available liquidity, fee handling for non-standard tokens, and the executor/mailbox permissioning that moves instructions across chains.
Findings
High
Medium
- BasicVault is incompatible with fee-on-transfer tokens
- Metadata is not set in Cap, which is required by the inherited Router to specify gas limit to the mailbox
- StrategyExecutor::disableStrategy() disables the wrong strategy
- RedeemQueue::getAvailableResolveRange() incorrectly returns false if from == count - 1
- BasicVault::_redeem() does not correctly deal with a disabled redeem queue after it was enabled
- BasicVault::_deposit() should always resolve with idle balance as the redeem queue may be disabled with requests pending
Low
- Protocol should disable renouncing ownership if it is never intended
- The StrategyExecutor can remain in a paused state if the owner renounces control while it's paused
- BasicVault::manualRedeem() and BasicVault::manualDeposit() are inconsistent
- BasicVault::getRedeemRequestOf() will revert as the redeem requests are filled with the wrong indexes
- Cap::receive() does not place restrictions in the sender, which may lead to donations
- _msgSender() is mixed with msg.sender
Informational
- Adopt named mappings for clarity
- Linea does not support PUSH0
- BasicVaultFactory::createVault() inconsistent already existing vault check
- Initializable constracts should call _disableInitializers() in the constructor instead of using the initializer modifier
- pragma abicoder v2; is turned on by default after a certain solidity version
- RedeemQueue::isResolved() resolvedCount always returns requestIds.length
- RedeemQueue::findOffsetIndex() does not check the last index
- Structs in RedeemQueue are out of order and should all be placed at the top
- RedeemQueue could be optimized
- RedeemQueue::get() reverts due to underflow when it should revert and throw the correct error
- Solidity types uint256 are never negative
- BasicVault::_resolveWithIdleBalance() can return before calculating _idleBalance() to save gas
- Storage variables can be cached to save gas
- decimals() is not part of the ERC20 standard and not all tokens may implement it as expected, which may cause initialize() to fail
- Cap::_checkRemoteStateAndAdvance() may be optimized by returning early as soon as a different epoch is found in a remote domain
- The domain with 0 index in Cap::_checkRemoteStateAndAdvance() is not checked for equality
- The current epoch will never advance if there are no other domains
- Cap::setEpochCap() may add a stale cap
- In AggregateHook, several if (newAmount == 0) checks are ambiguous
The report