From 4f84195871f7bc5eab8a05ebfbef33d68545ac26 Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Thu, 10 Apr 2025 17:34:33 -0300 Subject: [PATCH 1/2] feat: add resend msg feature changes on fmas (#12) --- security/fma-message-passing-contracts.md | 21 ++++--- security/fma-superchainerc20.md | 73 +++++++++++------------ security/fma-superchainethbridge.md | 60 +++++++++---------- 3 files changed, 77 insertions(+), 77 deletions(-) diff --git a/security/fma-message-passing-contracts.md b/security/fma-message-passing-contracts.md index 6c696a36..9bfc93d3 100644 --- a/security/fma-message-passing-contracts.md +++ b/security/fma-message-passing-contracts.md @@ -63,17 +63,22 @@ Below are references for this project: - **Detection:** Monitoring tools should track every validated message to ensure it was observed and validated at the origin chain. - **Recovery Path(s):** The chain will eventually resolve the issue as the dependency graph advances, but the resolution could be slow, especially if the sequencer is down or equivocating, or if chains in the dependency graph stop batching. Investigate the causes of such incidents. -### FM3: Valid message is initiated but never relayed in destination although is safe to include +### FM3: Valid message is initiated but never relayed in destination although it is safe to include - **Description:** A user (or contract) may send a valid cross-chain message, but the final relay step—`relayMessage` or `validateMessage`—never occurs (or is dropped before to gain a safe status) even when the initiated message is included in a finalized block on the origin chain. + + Some consequences of this issue are: + + - Inability to `relayETH` in `SuperchainETHBridge` + - Inability to `relayERC20` in `SuperchainTokenBridge` to mint `SuperchainERC20` + - Any other contract awaiting a time-sensitive `relayMessage`. + - **Risk Assessment:** High. - - Potential Impact: Critical. Several scenarios could lead to severe consequences, including: - - Inability to `relayETH` in `SuperchainWETH` - - Inability to `relayERC20` in `SuperchainTokenBridge` to mint `SuperchainERC20` - - Any other contract awaiting a time-sensitive `relayMessage`. - If the message is not included within the expiration window, both the message and the underlying action could be considered lost. - - Likelihood: Medium. Block builders/sequencers are generally controlled by a single entity per chain. -- **Mitigations:** From a smart contract perspective, little can be done except allowing calls to `validateMessage` within a deposit context to improve censorship resistance. This is currently under discussion [here](https://github.com/ethereum-optimism/specs/issues/520). + - Potential Impact: High. If the message failure is not related to a time-sensitive operation but is due to the message being incorrect, it could be considered lost. + - Likelihood: Medium. Block builders/sequencers are generally controlled by a single entity per chain. Additionally, if the relay failed on destination due to an incorrect state and after some time it is safe to be relayed again, you can do it even if it expired by call `resendMessage` on origin chain. +- **Mitigations:** + - If the problem is that the message is expired, calling `resendMessage` on origin chain to make it available again to be relayed. + - From a smart contract perspective, allowing calls to `validateMessage` within a deposit context to improve censorship resistance. This is currently under discussion [here](https://github.com/ethereum-optimism/specs/issues/520). - **Detection:** Monitoring tools should track whether every initiated message has been validated at the destination by checking identifiers. Support tickets filed by users reporting the issue sustain the severity of the case in some situations. - **Recovery Path(s):** Depends on sequencer/chain governor policy operations. diff --git a/security/fma-superchainerc20.md b/security/fma-superchainerc20.md index 8aa3c99c..2c3d0e7d 100644 --- a/security/fma-superchainerc20.md +++ b/security/fma-superchainerc20.md @@ -1,18 +1,18 @@ -## **Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* - +## **Table of Contents** _generated with [DocToc](https://github.com/thlorenz/doctoc)_ +- [**Table of Contents** _generated with DocToc_](#table-of-contents-generated-with-doctoc) - [**SuperchainERC20 standard-only FMA (Failure Modes and Recovery Path Analysis)**](#superchainerc20-standard-only-fma-failure-modes-and-recovery-path-analysis) - [Introduction](#introduction) - [Audit Requirements](#audit-requirements) - [Security Considerations](#security-considerations) - [Failure Modes and Recovery Paths](#failure-modes-and-recovery-paths) - - [FM1: Unauthorized Access to `crosschainMint` & `crosschainBurn` Functions](#fm1-unauthorized-access-to-crosschainmint--crosschainburn-functions) - - [FM2: Token Contract Missing in Destination Chain — Relay Fails Until Deployed (But Deployment Is Permissionless)](#fm2-token-contract-missing-in-destination-chain--relay-fails-until-deployed-but-deployment-is-permissionless) - - [FM3: Token Contract Missing in Destination Chain — Relay Fails Until Deployed (But Deployment Is Permissioned)](#fm3-token-contract-missing-in-destination-chain--relay-fails-until-deployed-but-deployment-is-permissioned) - - [FM4: Token Contract Missing in Destination Chain — Token Deployer Is Lost or Unable to Deploy to Expected Address](#fm4-token-contract-missing-in-destination-chain--token-deployer-is-lost-or-unable-to-deploy-to-expected-address) + - [FM1: Unauthorized Access to `crosschainMint` \& `crosschainBurn` Functions](#fm1-unauthorized-access-to-crosschainmint--crosschainburn-functions) + - [FM2: Token Contract Missing in Destination Chain: Relay Fails Until Deployed (But Deployment Is Permissioned)](#fm2-token-contract-missing-in-destination-chain-relay-fails-until-deployed-but-deployment-is-permissioned) + - [FM3: Token Contract Missing in Destination Chain: Relay Fails Until Deployed (But Deployment Is Permissionless)](#fm3-token-contract-missing-in-destination-chain-relay-fails-until-deployed-but-deployment-is-permissionless) + - [FM4: Token Contract Missing in Destination Chain: Token Deployer is Lost, or Unable to Deploy to Expected Address](#fm4-token-contract-missing-in-destination-chain-token-deployer-is-lost-or-unable-to-deploy-to-expected-address) - [FM5: Compromised Deployment Method](#fm5-compromised-deployment-method) - [Action Items](#action-items) - [Warning for Integrators](#warning-for-integrators) @@ -20,12 +20,12 @@ ## **SuperchainERC20 standard-only FMA (Failure Modes and Recovery Path Analysis)** -| Author | Ng, Joxes, Particle, Gotzen | -| --- | --- | -| Created at | 2024-10-02 | -| Needs Approval From | Mark Tyneway, Matt Solomon | -| Other Reviewers | Michael Amadi | -| Status | Implementing Actions | +| Author | Ng, Joxes, Particle, Gotzen | +| ------------------- | --------------------------- | +| Created at | 2024-10-02 | +| Needs Approval From | Mark Tyneway, Matt Solomon | +| Other Reviewers | Michael Amadi | +| Status | Implementing Actions | ## Introduction @@ -33,7 +33,7 @@ This document is intended to be shared publicly for review and visibility purpos Token deployers building on Superchain are encouraged to review the document, as they are the main beneficiaries of this standard. It does not intend to cover contracts such as SuperchainTokenBridge or those involving migrated liquidity. ->🗂️ The proposed implementation of the standard doesn’t prevent a token issuer from using other token standards, such as xERC20, with the `SuperchainTokenBridge` to mint and burn tokens across an [interoperable set of chains](https://specs.optimism.io/interop/overview.html). More about this is explained in [xERC20-ERC7802 compatilibility](https://defi-wonderland.notion.site/xERC20-ERC7802-compatibility-14c9a4c092c780ca94a8cb81e980d813). +> 🗂️ The proposed implementation of the standard doesn’t prevent a token issuer from using other token standards, such as xERC20, with the `SuperchainTokenBridge` to mint and burn tokens across an [interoperable set of chains](https://specs.optimism.io/interop/overview.html). More about this is explained in [xERC20-ERC7802 compatilibility](https://defi-wonderland.notion.site/xERC20-ERC7802-compatibility-14c9a4c092c780ca94a8cb81e980d813). Below are references for this project: @@ -52,10 +52,10 @@ Similar to ERC20, SuperchainERC20 implementations should be considered untrusted ### FM1: Unauthorized Access to `crosschainMint` & `crosschainBurn` Functions -- **Description:** The `crosschainMint` and `crosschainBurn` functions can only be called by the `SuperchainTokenBridge`, enforced by the check `msg.sender != Predeploys.SUPERCHAIN_TOKEN_BRIDGE`. If the bridge address is badly defined or the modifier bypassed, an entity could mint and burn tokens. +- **Description:** The `crosschainMint` and `crosschainBurn` functions can only be called by the `SuperchainTokenBridge`, enforced by the check `msg.sender != Predeploys.SUPERCHAIN_TOKEN_BRIDGE`. If the bridge address is badly defined or the modifier bypassed, an entity could mint and burn tokens. - **Risk Assessment**: Medium. - - Potential impact: High. All tokens based on this implementation could be potentially at risk. - - Likelihood: Very Low. `Predeploys.SUPERCHAIN_TOKEN_BRIDGE` is defined via protocol upgrades. The conditional is sufficiently simple and battle-tested to give confidence in the implementation. + - Potential impact: High. All tokens based on this implementation could be potentially at risk. + - Likelihood: Very Low. `Predeploys.SUPERCHAIN_TOKEN_BRIDGE` is defined via protocol upgrades. The conditional is sufficiently simple and battle-tested to give confidence in the implementation. - **Mitigation**: Current unit tests perform authorization checks for `crosschainMint` ([test](https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/test/L2/SuperchainERC20.t.sol#L38)) and `crosschainBurn` ([test](https://github.com/ethereum-optimism/optimism/blob/1add88a41b8cdc5c488a18976e3e4fa4b02e7da4/packages/contracts-bedrock/test/L2/SuperchainERC20.t.sol#L77)). There should be tests to ensure that the `SuperchainTokenBridge` predeploy is correctly set during the interop fork upgrade, and is not subject to unexpected changes. - **Detection**: Ideally, run an off-chain agent to monitor every `crosschainMint` and `crosschainBurn` event in real-time, perhaps it could be resource-intensive given the permissionless usage of `SuperchainERC20` and the expected large number of deployments. If that’s not feasible, rely on user-filed support tickets to flag unusual mint/burn activity, which the TBD team (see corresponding action item) can investigate for suspected unauthorized access. - **Recovery Path(s)**: Equivocation (i.e. another implementation being set) on the `SuperchainTokenBridge` address would require a protocol upgrade or hard fork to restore the expected code. @@ -63,11 +63,9 @@ Similar to ERC20, SuperchainERC20 implementations should be considered untrusted ### FM2: Token Contract Missing in Destination Chain: Relay Fails Until Deployed (But Deployment Is Permissioned) - **Description**: The token on the destination chain is not yet deployed, which prevents the cross-chain transfer from finalizing (`relayERC20` fails). However, the token's deployment is permissioned, meaning it is solely up to the deployer to make the token available on the destination chain -- **Risk Assessment**: High. - - Potential impact: High. Users may lose access to their tokens temporarily or permanently. Two specific scenarios can arise: - 1. Funds are stuck but can eventually be relayed if the deployment occurs before the message expiration window ends. - 2. Funds are stuck and potentially lost if the deployment occurs after the message expiration time ends or never occurs. - - Likelihood: Medium. A deployer may choose not to deploy the token on all chains. +- **Risk Assessment**: Medium. + - Potential impact: Medium. Users may lose access to their tokens temporarily, until the token is deployed. + - Likelihood: Medium. A deployer may choose not to deploy the token on all chains. - **Mitigation**: Trusted bridge frontends should prevent users from sending tokens to a chain where the token doesn’t exist. Double-check other trusted sources (such as Superchain Token List) for greater confidence. - **Detection**: Support tickets filed by users reporting the issue. - **Recovery Path(s)**: Communicate with the token owner and request deployment of the token on the missing chain. @@ -76,19 +74,18 @@ Similar to ERC20, SuperchainERC20 implementations should be considered untrusted - **Description**: The token on the destination chain is not yet deployed, which prevents the cross-chain transfer from finalizing (`relayERC20` fails). However, the token is permissionless to deploy, meaning anyone can deploy it. - **Risk Assessment**: Medium - - Potential impact: Medium. Users are unable to access their tokens until the token is deployed. - - Likelihood: Medium. Tokens may not be deployed on all chains. + - Potential impact: Medium. Users are unable to access their tokens until the token is deployed. + - Likelihood: Medium. Tokens may not be deployed on all chains. - **Mitigation**: Trusted bridge frontends should prevent users from sending tokens to any chain where the token does not exist. It is recommended to deploy tokens on every new chain added to the dependency set. - **Detection**: Support tickets filed by users reporting the issue. - **Recovery Path(s)**: Deploy the token. - ### FM4: Token Contract Missing in Destination Chain: Token Deployer is Lost, or Unable to Deploy to Expected Address - **Description**: For the `SuperchainTokenBridge` to validate cross-chain mints and burns correctly, the `SuperchainERC20` must appear at one consistent address on each chain when deployed. If a developer fails to deterministically deploy the token at the same address, the bridging logic cannot unify the token references across chains, leading to failed or incorrect cross-chain transfers. - **Risk Assessment**: Medium - - Potential impact: High. Without a consistent address, interoperability for the token is effectively disabled and `relayERC20` are likely to fail, causing a lost. - - Likelihood: Very Low. The interoperable set of chains follows the same opcode behavior and ensures identical availability of deployer contracts, such as `create2Deployer`. + - Potential impact: High. Without a consistent address, interoperability for the token is effectively disabled and `relayERC20` are likely to fail, causing a lost. + - Likelihood: Very Low. The interoperable set of chains follows the same opcode behavior and ensures identical availability of deployer contracts, such as `create2Deployer`. - **Mitigation**: For developers, employ the appropriate deterministic deployment tools, such as the one at `create2Deployer`. For users, refers to FM2 and FM3. - **Detection**: Deployers should test the deployment method used and verify contract addresses after a mainnet deployment. - **Recovery Path(s)**: Redeploy token contracts on a new address across chains and migrate userbase if it is needed. @@ -97,10 +94,10 @@ Similar to ERC20, SuperchainERC20 implementations should be considered untrusted - **Description**: The bridging logic assumes that if a `SuperchainERC20` contract address is identical across multiple chains, it refers to the expected code. However, if a deployer uses flawed or deployment methods that get compromised, a non-desired implementation may be deployed at the same address on one chain. - **Risk Assessment**: Medium - - Potential impact: High. In the worst-case scenario, compromised methods open the door to malicious behavior, e.g. where an attacker can occupy the address, and forge bridging events, and gain the ability to mint tokens in other chains. - - Likelihood: Low. This is possible if the deployer engages in poor practices or lacks opsec. Two examples of this are: - - Custom factories that utilize CREATE and share the same address are used, where tokens are deployed using the same bytecode and nonce. - - Proxy contracts where the admin is compromised or the proxy address front-runned. + - Potential impact: High. In the worst-case scenario, compromised methods open the door to malicious behavior, e.g. where an attacker can occupy the address, and forge bridging events, and gain the ability to mint tokens in other chains. + - Likelihood: Low. This is possible if the deployer engages in poor practices or lacks opsec. Two examples of this are: + - Custom factories that utilize CREATE and share the same address are used, where tokens are deployed using the same bytecode and nonce. + - Proxy contracts where the admin is compromised or the proxy address front-runned. - **Mitigation**: For developers, employ the appropriate deterministic deployment tools, such as the one at `create2Deployer`. In the case of permissioned deployments, common for proxy contracts, ensure to maintain control over those deployments and ownership. - **Detection**: Support tickets filed by users reporting such issues. - **Recovery Path(s)**: Pause the token contract where possible and proceed to redeploy and migrate userbase if it is needed. @@ -109,13 +106,13 @@ Similar to ERC20, SuperchainERC20 implementations should be considered untrusted The following action items need to be done: -- [ ] Resolve all the comments. -- [ ] FM1: Implement tests for `SuperchainTokenBridge` predeploy. Currently, the L2 Genesis project is under development and it should take care of these tests. -- [ ] FM1: Run an off-chain script to monitor `crosschainMint` and `crosschainBurn` events for parity and consistency with the expected `msg.sender` (`SuperchainTokenBridge`). (Owned by OP Labs) -- [ ] FM1: Communicate with the team responsible for responding to alerts about the need to define how issues will be raised to the security team. -- [ ] FM2, FM3, FM4: Communicate to bridge frontends to ensure they prevent users from sending tokens to a chain where the token doesn’t exist. -- [ ] FM4, FM5: Ensure that the documentation for SuperchainERC20 developers explains the need for deterministic deployment and how to achieve it, including that constructor args affect the resulting address, requiring consistency across chains. -- [ ] FM1, FM2, FM3, FM5: Communicate with the team in charge of responding to user-submitted support tickets about the need to create runbooks and define how issues are escalated to the security team or returned to the user when security involvement is not required. +- [ ] Resolve all the comments. +- [ ] FM1: Implement tests for `SuperchainTokenBridge` predeploy. Currently, the L2 Genesis project is under development and it should take care of these tests. +- [ ] FM1: Run an off-chain script to monitor `crosschainMint` and `crosschainBurn` events for parity and consistency with the expected `msg.sender` (`SuperchainTokenBridge`). (Owned by OP Labs) +- [ ] FM1: Communicate with the team responsible for responding to alerts about the need to define how issues will be raised to the security team. +- [ ] FM2, FM3, FM4: Communicate to bridge frontends to ensure they prevent users from sending tokens to a chain where the token doesn’t exist. +- [ ] FM4, FM5: Ensure that the documentation for SuperchainERC20 developers explains the need for deterministic deployment and how to achieve it, including that constructor args affect the resulting address, requiring consistency across chains. +- [ ] FM1, FM2, FM3, FM5: Communicate with the team in charge of responding to user-submitted support tickets about the need to create runbooks and define how issues are escalated to the security team or returned to the user when security involvement is not required. ## Warning for Integrators @@ -129,4 +126,4 @@ The reason why this should not be overlooked is that otherwise protocols can wro Although this is a vector of attack that is true for all tokens, the proclivity of `SuperchainERC20s` to share their address make the attack more likely as the attacker can more easily predict the address of a token that a given protocol may support in new chains. -As an example, if `SuperchainERC20_A` is doing well in `Protocol_A` in Optimism, and `Protocol_A` decides to launch on Unichain as well, it's likely it will eventually support `SuperchainERC20_A` in there as well. With this, the attacker can get ahead and increase his balance in Unichain before `SuperchainERC20_A` is deployed there. \ No newline at end of file +As an example, if `SuperchainERC20_A` is doing well in `Protocol_A` in Optimism, and `Protocol_A` decides to launch on Unichain as well, it's likely it will eventually support `SuperchainERC20_A` in there as well. With this, the attacker can get ahead and increase his balance in Unichain before `SuperchainERC20_A` is deployed there. diff --git a/security/fma-superchainethbridge.md b/security/fma-superchainethbridge.md index 6728ea97..2d095dd5 100644 --- a/security/fma-superchainethbridge.md +++ b/security/fma-superchainethbridge.md @@ -1,58 +1,56 @@ # SuperchainETHBridge: Failure Modes and Recovery Path Analysis -| Author | Joxes, Gotzen, Parti | -| --- | --- | -| Created at | 2025-01-10 | -| Initial Reviewers | Kelvin Fichter, Michael Amadi | -| Needs Approval From | Kelvin Fichter | -| Status | Implementing Actions | +| Author | Joxes, Gotzen, Parti | +| ------------------- | ----------------------------- | +| Created at | 2025-01-10 | +| Initial Reviewers | Kelvin Fichter, Michael Amadi | +| Needs Approval From | Kelvin Fichter | +| Status | Implementing Actions | ## Introduction This document covers the addition of the `SuperchainETHBridge` contract, an abstraction layer on top of the `L2toL2CrossDomainMessenger` specifically designed for native ETH transfers between chains. The following components are included: - **Contracts**: - - Introducing the `SuperchainETHBridge`predeploy proxy and implementation contract. It uses the `L2toL2CrossDomainMessenger` to send and relay messages that handle ETH transfers between chains. - - Introducing the `ETHLiquidity` predeploy proxy and implementation contract. It is designed to provide the ETH for the `SuperchainETHBridge` to facilitate ETH transfers between chains, by maintaining a large reserve of native ETH. + - Introducing the `SuperchainETHBridge`predeploy proxy and implementation contract. It uses the `L2toL2CrossDomainMessenger` to send and relay messages that handle ETH transfers between chains. + - Introducing the `ETHLiquidity` predeploy proxy and implementation contract. It is designed to provide the ETH for the `SuperchainETHBridge` to facilitate ETH transfers between chains, by maintaining a large reserve of native ETH. Below are references for this project: - Design doc: - - Introduction of SuperchainETHBridge: https://github.com/ethereum-optimism/design-docs/blob/main/protocol/superchain-eth-bridge.md - - Handling ETH transfers: https://github.com/ethereum-optimism/design-docs/blob/main/protocol/interoperable-ether-transfers.md -- Specs: - - `SuperchainETHBridge`: https://github.com/ethereum-optimism/specs/blob/main/specs/interop/superchain-eth-bridge.md - - `ETHLiquidity`: https://github.com/ethereum-optimism/specs/blob/main/specs/interop/eth-liquidity.md + - Introduction of SuperchainETHBridge: https://github.com/ethereum-optimism/design-docs/blob/main/protocol/superchain-eth-bridge.md + - Handling ETH transfers: https://github.com/ethereum-optimism/design-docs/blob/main/protocol/interoperable-ether-transfers.md +- Specs: + - `SuperchainETHBridge`: https://github.com/ethereum-optimism/specs/blob/main/specs/interop/superchain-eth-bridge.md + - `ETHLiquidity`: https://github.com/ethereum-optimism/specs/blob/main/specs/interop/eth-liquidity.md - Implementation: - - `SuperchainETHBridge`: https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L2/SuperchainETHBridge.sol - - `ETHLiquidity`: https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L2/ETHLiquidity.sol + - `SuperchainETHBridge`: https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L2/SuperchainETHBridge.sol + - `ETHLiquidity`: https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L2/ETHLiquidity.sol - ->🗂️ **For more context about the Interop project, refer to the following docs:** +> 🗂️ **For more context about the Interop project, refer to the following docs:** +> > 1. [Interop System Diagram](https://www.notion.so/16c8052fcbb24b93ad1a539b5f8db4c1?pvs=21) > 2. [Interop PID](https://www.notion.so/16c8052fcbb24b93ad1a539b5f8db4c1?pvs=21) > 3. [Interop Audit Request](https://docs.google.com/document/d/1Rcuzbsguh7koT2jFru5ft9T8zAvjBEzbt0zF5LNQQ08/edit?tab=t.0) - - ## Failure Modes and Recovery Paths ### FM1: Insufficient ETH in `ETHLiquidity` - **Description:** If `ETHLiquidity` lacks sufficient ETH, mint calls will revert when the contract attempts to forward funds using `SafeSend`. This breaks bridging flows dependent on `relayETH`. - **Risk Assessment:** Medium. - - Potential Impact: High. Not having enough ETH balance in `ETHLiquidity` prevents relaying ETH, which would greatly degrade the user experience, as they would be temporarily stuck. The situation becomes even more serious if the affected cross-chain messages expire before the issue can be resolved, making it impossible to retry the relay and get the funds in destination chain. - - Likelihood: Very low. `ETHLiquidity` starts with a maximum balance (`type(uint248).max`). + - Potential Impact: High. Not having enough ETH balance in `ETHLiquidity` prevents relaying ETH, which would greatly degrade the user experience, as they would be temporarily stuck. + - Likelihood: Very low. `ETHLiquidity` starts with a maximum balance (`type(uint248).max`). - **Mitigations:** Our current codebase includes tests to check if the `mint` request exceeds the contract’s balance ([test](https://github.com/ethereum-optimism/optimism/blob/dd37e6192c37ed4c5b18df0269f065f378c495cc/packages/contracts-bedrock/test/L2/ETHLiquidity.t.sol#L103)). Initial ETH supply in `ETHLiquidity` is `type(uint248).max` ([test](https://github.com/ethereum-optimism/optimism/blob/dd37e6192c37ed4c5b18df0269f065f378c495cc/packages/contracts-bedrock/test/L2/ETHLiquidity.t.sol#L29)) and equally set in all chains. - **Detection:** Perform checks on the `ETHLiquidity` balance as a preventive monitoring measure. User-filed support tickets may flag this issue in case of failure. -- **Recovery Path(s):** Coordinate an L2 hard fork to replenish the `ETHLiquidity` contract. Investigate the causes of the depletion to determine if other factors are involved. Messages will need to be retried for relaying; otherwise, use the `expireMessage` feature if it is available and integrated into `SuperchainETHBridge`. +- **Recovery Path(s):** Coordinate an L2 hard fork to replenish the `ETHLiquidity` contract. Investigate the causes of the depletion to determine if other factors are involved. Messages will need to be retried for relaying, leveraging the `resendMessage` feature if needed. ### FM2: Overflow during `burn` in `ETHLiquidity` due to max Balance - **Description:** If the `ETHLiquidity` contract has already reached the maximum allowed ETH balance (`type(uint256).max`), invoking the `sendETH` function could cause an overflow. This occurs when `burn` is called with an amount that, when added to the current balance, exceeds the maximum `uint256` value. Such an overflow would cause a revert, since the proper Solidity version is used. - **Risk Assessment:** Low. - - Potential Impact: Medium. Reverts during `sendETH` calls are expected when the requested amount exceeds the maximum value representable by a `uint256`. - - Likelihood: Very low. `ETHLiquidity` starts with a maximum balance (`type(uint248).max`) which is still far from `uint256` limit. + - Potential Impact: Medium. Reverts during `sendETH` calls are expected when the requested amount exceeds the maximum value representable by a `uint256`. + - Likelihood: Very low. `ETHLiquidity` starts with a maximum balance (`type(uint248).max`) which is still far from `uint256` limit. - **Mitigations:** Initial ETH supply in `ETHLiquidity` is `type(uint248).max` properly is checked ([test](https://github.com/ethereum-optimism/optimism/blob/dd37e6192c37ed4c5b18df0269f065f378c495cc/packages/contracts-bedrock/test/L2/ETHLiquidity.t.sol#L29)). - **Detection:** Perform checks on the `ETHLiquidity` balance as a preventive monitoring measure. User-filed support tickets may flag this issue in case of failure. - **Recovery Path(s):** Coordinate an L2 hard fork to adjust the `ETHLiquidity` contract’s state. Investigate the causes of this failure to determine if other factors are involved. @@ -61,18 +59,18 @@ Below are references for this project: See [fma-generic-contracts.md](https://github.com/ethereum-optimism/design-docs/blob/main/security/fma-generic-contracts.md). -- [x] Check this box to confirm that these items have been considered and updated if necessary. +- [x] Check this box to confirm that these items have been considered and updated if necessary. -See [relevant FMAs to SuperchainETHBridge, To Do] +See [relevant FMAs to SuperchainETHBridge, To Do] -- [ ] Check this box to confirm that these items have been considered and updated if necessary. +- [ ] Check this box to confirm that these items have been considered and updated if necessary. ## Action Items -- [x] Resolve all the comments. -- [ ] FM1, FM2: Establish a balance monitoring measure in `ETHLiquidity` (optional). -- [ ] Ensure the support team is aware of these failure modes and prepared to respond. -- [ ] **Ensure that the actions items specified in each FMAs on which SuperchainETHBridge depends are completed.** +- [x] Resolve all the comments. +- [ ] FM1, FM2: Establish a balance monitoring measure in `ETHLiquidity` (optional). +- [ ] Ensure the support team is aware of these failure modes and prepared to respond. +- [ ] **Ensure that the actions items specified in each FMAs on which SuperchainETHBridge depends are completed.** ## Audit Requirements From e8fd593fc74c12272d13da7db8c8963885aeb39d Mon Sep 17 00:00:00 2001 From: Disco <131301107+0xDiscotech@users.noreply.github.com> Date: Mon, 21 Apr 2025 15:26:14 -0300 Subject: [PATCH 2/2] fix: fma three (#17) --- security/fma-message-passing-contracts.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/security/fma-message-passing-contracts.md b/security/fma-message-passing-contracts.md index 7abc5c10..9f6ac7ce 100644 --- a/security/fma-message-passing-contracts.md +++ b/security/fma-message-passing-contracts.md @@ -63,23 +63,25 @@ Below are references for this project: - **Detection:** Monitoring tools should track every validated message to ensure it was observed and validated at the origin chain. - **Recovery Path(s):** The chain will eventually resolve the issue as the dependency graph advances, but the resolution could be slow, especially if the sequencer is down or equivocating, or if chains in the dependency graph stop batching. Investigate the causes of such incidents. -### FM3: Valid message is initiated but never relayed in destination although it is safe to include +### FM3: Valid message is initiated but never relayed in destination due to sequencer censorship or inaction -- **Description:** A user (or contract) may send a valid cross-chain message, but the final relay step—`relayMessage` or `validateMessage`—never occurs (or is dropped before to gain a safe status) even when the initiated message is included in a finalized block on the origin chain. - - Some consequences of this issue are: +- **Description:** A user (or contract) may send a valid cross-chain message, but the final relay step—`relayMessage` or `validateMessage`—never occurs. This can happen even if the message is included in a finalized block on the origin chain. In some cases, a sequencer on the destination chain may intentionally choose not to relay the message (e.g., if the origin chain is not trusted or politically unfavorable to the operator). +- **Consequences:** - Inability to `relayETH` in `SuperchainETHBridge` - Inability to `relayERC20` in `SuperchainTokenBridge` to mint `SuperchainERC20` - Any other contract awaiting a time-sensitive `relayMessage`. -- **Risk Assessment:** High. - - Potential Impact: High. If the message failure is not related to a time-sensitive operation but is due to the message being incorrect, it could be considered lost. - - Likelihood: Medium. Block builders/sequencers are generally controlled by a single entity per chain. Additionally, if the relay failed on destination due to an incorrect state and after some time it is safe to be relayed again, you can do it even if it expired by call `resendMessage` on origin chain. +- **Risk Assessment:** High + - Potential Impact: High. Time-sensitive or critical messages may never be processed. + - Likelihood: Medium. Block builders/sequencers are generally controlled by a single entity per chain. + - **Mitigations:** - - If the problem is that the message is expired, calling `resendMessage` on origin chain to make it available again to be relayed. + - Use `resendMessage` on the origin chain to re-emit the event and allow re-processing in case the time-window has expired. - From a smart contract perspective, allowing calls to `validateMessage` within a deposit context to improve censorship resistance. This is currently under discussion [here](https://github.com/ethereum-optimism/specs/issues/520). + - **Detection:** Monitoring tools should track whether every initiated message has been validated at the destination by checking identifiers. Support tickets filed by users reporting the issue sustain the severity of the case in some situations. + - **Recovery Path(s):** Depends on sequencer/chain governor policy operations. ### FM4: Invalid Replayed Message (Replay attack) in `L2ToL2CrossDomainMessenger`