Commit 43de7fc4 authored by Maurelian's avatar Maurelian Committed by GitHub

Disallow relayMessage to L2 to L1 Message Passer (#360)

* prevent call spoofing on L2
* adds a test based on logs and successfulMessages mapping
parent 1378674b
......@@ -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
......
......@@ -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,15 @@ 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;
......
......@@ -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);
......
......@@ -13,6 +13,7 @@ import {
NON_ZERO_ADDRESS,
getXDomainCalldata,
} from '../../../../helpers'
import { solidityKeccak256 } from 'ethers/lib/utils'
describe('OVM_L2CrossDomainMessenger', () => {
let signer: Signer
......@@ -157,5 +158,44 @@ describe('OVM_L2CrossDomainMessenger', () => {
OVM_L2CrossDomainMessenger.relayMessage(target, sender, message, 0)
).to.be.revertedWith('Provided message has already been received.')
})
it('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]
)
const resProm = OVM_L2CrossDomainMessenger.relayMessage(
target,
sender,
message,
0
)
// The call to relayMessage() should succeed.
await expect(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
})
})
})
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment