Disallow relayMessage to L2 to L1 Message Passer

This change prevents call spoofing on L2.
Adds a test based on logs and successfulMessages mapping.

Author: maurelian

contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol

@@ -19,9 +19,9 @@ import { Abs_BaseCrossDomainMessenger } from "./Abs_BaseCrossDomainMessenger.sol
 
 /**
  * @title OVM_L1CrossDomainMessenger
- * @dev The L1 Cross Domain Messenger contract sends messages from L1 to L2, and relays messages from L2 onto L1. 
- * In the event that a message sent from L1 to L2 is rejected for exceeding the L2 epoch gas limit, it can be resubmitted 
- * via this contract's replay function. 
+ * @dev The L1 Cross Domain Messenger contract sends messages from L1 to L2, and relays messages from L2 onto L1.
+ * In the event that a message sent from L1 to L2 is rejected for exceeding the L2 epoch gas limit, it can be resubmitted
+ * via this contract's replay function.
  *
  * Compiler used: solc
  * Runtime target: EVM

contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L2CrossDomainMessenger.sol

@@ -18,7 +18,7 @@ import { Abs_BaseCrossDomainMessenger } from "./Abs_BaseCrossDomainMessenger.sol
  * @title OVM_L2CrossDomainMessenger
  * @dev The L2 Cross Domain Messenger contract sends messages from L2 to L1, and is the entry point
  * for L2 messages sent via the L1 Cross Domain Messenger.
- * 
+ *
  * Compiler used: optimistic-solc
  * Runtime target: OVM
   */
@@ -75,6 +75,16 @@ contract OVM_L2CrossDomainMessenger is iOVM_L2CrossDomainMessenger, Abs_BaseCros
             "Provided message has already been received."
         );
 
+        // Prevent calls to OVM_L2ToL1MessagePasser, which would enable
+        // an attacker to maliciously craft the _message to spoof
+        // a call from any L2 account.
+        if(_target == resolve("OVM_L2ToL1MessagePasser")){
+            // Write to the successfulMessages mapping and return immediately.
+            successfulMessages[xDomainCalldataHash] = true;
+            return;
+        }
+
+
         xDomainMsgSender = _sender;
         (bool success, ) = _target.call(_message);
         xDomainMsgSender = DEFAULT_XDOMAIN_SENDER;

contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

@@ -159,7 +159,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
         override
         public
     {
-        require(transactionContext.ovmNUMBER == 0, "Only be callable at the start of a transaction");
+        require(transactionContext.ovmNUMBER == 0, "Only callable at the start of a transaction");
         // Store our OVM_StateManager instance (significantly easier than attempting to pass the
         // address around in calldata).
         ovmStateManager = iOVM_StateManager(_ovmStateManager);
@@ -640,7 +640,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
     {
         // DELEGATECALL does not change anything about the message context.
         MessageContext memory nextMessageContext = messageContext;
-        
+
         return _callContract(
             nextMessageContext,
             _gasLimit,
@@ -915,7 +915,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
     /**
      * Handles all interactions which involve the execution manager calling out to untrusted code (both calls and creates).
      * Ensures that OVM-related measures are enforced, including L2 gas refunds, nuisance gas, and flagged reversions.
-     * 
+     *
      * @param _nextMessageContext Message context to be used for the external message.
      * @param _gasLimit Amount of gas to be passed into this message.
      * @param _contract OVM address being called or deployed to
@@ -1028,7 +1028,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
      * Handles the creation-specific safety measures required for OVM contract deployment.
      * This function sanitizes the return types for creation messages to match calls (bool, bytes).
      * This allows for consistent handling of both types of messages in _handleExternalMessage().
-     * 
+     *
      * @param _gasLimit Amount of gas to be passed into this creation.
      * @param _creationCode Code to pass into CREATE for deployment.
      * @param _address OVM address being deployed to.
@@ -1075,7 +1075,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
 
         // Actually execute the EVM create message,
         address ethAddress = Lib_EthUtils.createContract(_creationCode);
-        
+
         if (ethAddress == address(0)) {
             // If the creation fails, the EVM lets us grab its revert data. This may contain a revert flag
             // to be used above in _handleExternalMessage.
@@ -1851,7 +1851,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
             if (created == address(0)) {
                 return (false, revertData);
             } else {
-                // The eth_call RPC endpoint for to = undefined will return the deployed bytecode 
+                // The eth_call RPC endpoint for to = undefined will return the deployed bytecode
                 // in the success case, differing from standard create messages.
                 return (true, Lib_EthUtils.getCode(created));
             }

test/contracts/OVM/bridge/base/OVM_L2CrossDomainMessenger.spec.ts

@@ -13,6 +13,7 @@ import {
   NON_ZERO_ADDRESS,
   getXDomainCalldata,
 } from '../../../../helpers'
+import { solidityKeccak256 } from 'ethers/lib/utils'
 
 describe('OVM_L2CrossDomainMessenger', () => {
   let signer: Signer
@@ -146,16 +147,39 @@ describe('OVM_L2CrossDomainMessenger', () => {
       ).to.be.revertedWith('xDomainMessageSender is not set')
     })
 
-    it('should revert if trying to send the same message twice', async () => {
+    it.only('should not make a call if the target is the L2 MessagePasser', async () => {
       Mock__OVM_L1MessageSender.smocked.getL1MessageSender.will.return.with(
         Mock__OVM_L1CrossDomainMessenger.address
       )
+      target = await AddressManager.getAddress('OVM_L2ToL1MessagePasser')
+      message = Mock__OVM_L2ToL1MessagePasser.interface.encodeFunctionData('passMessageToL1(bytes)', [
+        NON_NULL_BYTES32,
+      ])
 
-      await OVM_L2CrossDomainMessenger.relayMessage(target, sender, message, 0)
+      const resProm = OVM_L2CrossDomainMessenger.relayMessage(target, sender, message, 0)
 
+      // The call to relayMessage() should succeed.
       await expect(
-        OVM_L2CrossDomainMessenger.relayMessage(target, sender, message, 0)
-      ).to.be.revertedWith('Provided message has already been received.')
+        resProm
+      ).to.not.be.reverted
+
+      // There should be no 'relayedMessage' event logged in the receipt.
+      const logs = (
+        await Mock__OVM_L2ToL1MessagePasser.provider.getTransactionReceipt(
+          (await resProm).hash
+        )
+      ).logs
+      expect(logs).to.deep.equal([])
+
+      // The message should be registered as successful.
+      expect(
+        await OVM_L2CrossDomainMessenger.successfulMessages(
+          solidityKeccak256(
+            ["bytes"],
+            [getXDomainCalldata(await signer.getAddress(), target, message, 0)]
+          )
+        )
+      ).to.be.true
     })
   })
 })