Legacy Message Can Be Replayed After Migration Leading To Double-Spend Bug
Summary
There is a bug while computing the old hash of the legacy message that allows legacy messages to be replayed. This allows anyone who has bridged assets from L2 to L1 before the migration to bridge the assets again for the second time after the migration, effectively resulting in a double-spend bug.
Vulnerability Detail
The following is the new relayMessage function in Bedrock's L1CrossDomainMessenger contract.
File: CrossDomainMessenger.sol
256: function relayMessage(
257: uint256 _nonce,
258: address _sender,
259: address _target,
260: uint256 _value,
261: uint256 _minGasLimit,
262: bytes calldata _message
263: ) external payable nonReentrant whenNotPaused {
264: (, uint16 version) = Encoding.decodeVersionedNonce(_nonce);
265: require(
266: version < 2,
267: "CrossDomainMessenger: only version 0 or 1 messages are supported at this time"
268: );
..SNIP..
Note that the _nonce passed into the new relayMessage function is a versioned Nonce that consists of two components (versioning and nonce). The format of the versioned Nonce is as follows:
0xVVNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN
V = Version Info, N = Nonce Counter
During the migration, the LegacyWithdrawal is converted into the new bedrock-style Withdrawal.
The legacy-style nonce in the LegacyWithdrawal consists only of an Integer Nonce counter without any versioning information. During the conversion, it was replaced with a versioned Nonce. This is done for both the inner relayMessage payload and outer withdrawal payload. Refer to Lines 69, 74, and 89 below
File: migrate.go
52: // MigrateWithdrawal will turn a LegacyWithdrawal into a bedrock
53: // style Withdrawal.
54: func MigrateWithdrawal(withdrawal *LegacyWithdrawal, l1CrossDomainMessenger *common.Address) (*Withdrawal, error) {
..SNIP..
66: // Migrated withdrawals are specified as version 0. Both the
67: // L2ToL1MessagePasser and the CrossDomainMessenger use the same
68: // versioning scheme. Both should be set to version 0
69: versionedNonce := EncodeVersionedNonce(withdrawal.Nonce, new(big.Int))
70: // Encode the call to `relayMessage` on the `CrossDomainMessenger`.
71: // The minGasLimit can safely be 0 here.
72: data, err := abi.Pack(
73: "relayMessage",
74: versionedNonce,
75: withdrawal.Sender,
76: withdrawal.Target,
77: value,
78: new(big.Int),
79: withdrawal.Data,
80: )
..SNIP..
88: w := NewWithdrawal(
89: versionedNonce,
90: &predeploys.L2CrossDomainMessengerAddr,
91: l1CrossDomainMessenger,
92: value,
93: new(big.Int).SetUint64(gasLimit),
94: data,
95: )
96: return w, nil
The following is the new relayMessage function in the Bedrock's L1CrossDomainMessenger contract. Line 272-278 performs validation to ensure that the legacy version of the message has not already been relayed. It does this by performing the following actions:
1) Decoding the _nonce provided to determine the version.
2) If the nonce's version is zero, this means that it is a legacy withdrawal. Compute the old hash by calling Hashing.hashCrossDomainMessageV0 function with the _target, _sender, _message and _nonce
3) Verify if the old hash of the legacy message is set to true in the successfulMessages mapping. If it is true, this means that the message has already been relayed once before the migration, and the transaction will be reverted to prevent the legacy message from replaying in Bedrock.
The issue here is that the Hashing.hashCrossDomainMessageV0 expects the legacy-style nonce (without versioning) that contains only the Integer Nonce counter. However, the code passes a versioned nonce into the Hashing.hashCrossDomainMessageV0 function to compute the old hash of the legacy message. As a result, the hash computed will not match the actual old hash of the legacy message, and successfulMessages[oldHash] will return false. This allows legacy message that has been relayed before migration to be relayed again in Bedrock after the migration.
File: CrossDomainMessenger.sol
256: function relayMessage(
257: uint256 _nonce,
258: address _sender,
259: address _target,
260: uint256 _value,
261: uint256 _minGasLimit,
262: bytes calldata _message
263: ) external payable nonReentrant whenNotPaused {
264: (, uint16 version) = Encoding.decodeVersionedNonce(_nonce);
265: require(
266: version < 2,
267: "CrossDomainMessenger: only version 0 or 1 messages are supported at this time"
268: );
269:
270: // If the message is version 0, then it's a migrated legacy withdrawal. We therefore need
271: // to check that the legacy version of the message has not already been relayed.
272: if (version == 0) {
273: bytes32 oldHash = Hashing.hashCrossDomainMessageV0(_target, _sender, _message, _nonce);
274: require(
275: successfulMessages[oldHash] == false,
276: "CrossDomainMessenger: legacy withdrawal already relayed"
277: );
278: }
Proof-of-Concept
Assume that Mallory is the attacker attempting to exploit this issue, and Mallory's wallet address in both L1 and L2 is 0x536fbBaE279fd77FAe3E29b410f7B605bf45BC8b.
Steps to be done before Bedrock migration:
1) Mallory bridges 100 ETH from L2 to L1 by calling the L2StandardBridge.withdraw function
2) The L2StandardBridge will call the L2CrossDomainMessenger to send a cross-domain withdrawal message to L1CrossDomainMessenger
3) The L2CrossDomainMessenger will call the OVM_L2ToL1MessagePasser.passMessageToL1 to store the hash of the cross-domain withdrawal message in the storage of sentMessages mapping. The sentMessages mapping on the OVM_L2ToL1MessagePasser.passMessageToL1 will be used by the L1CrossDomainMessenger._verifyStorageProof() function at L1 to determine if a message has been sent on L2.
4) Once the cross-domain message gets relayed to L1, the L1CrossDomainMessenger.relayMessage function will be triggered. The L1CrossDomainMessenger will call the L1StandardBridge.finalizeETHWithdrawal to transfer 100 ETH to Mallory's address in L1.
5) After the transfer is done, the successfulMessages[xDomainCalldataHash] is set to true to prevent the message from being replayed. Note that the key of the successfulMessages is derived from the xDomainCalldataHash. The xDomainCalldataHash is generated via the following algorithm:
xDomainCalldataHash = keccak256(
abi.encodeWithSignature(
"relayMessage(address,address,bytes,uint256)",
_target,
_sender,
_message,
_messageNonce <=== this is "999"
)
)
Assume that the _messageNonce here is 999. Thus, the 999 value is one of the inputs used to compute the hash. Assume that the generated xDomainCalldataHash is c89efdaa54c0f20c7adf612882df0950f5a951637e0307cdcb4c672f298b8bc6.
As such, successfulMessages[c89efdaa54c0f20c7adf612882df0950f5a951637e0307cdcb4c672f298b8bc6] = true.
During the migration:
During the migration, all legacy withdrawals from old OVM_L2ToL1MessagePasser will be converted to new bedrock-style withdrawals and stored in the sentMessages mapping of the new L2ToL1MessagePasser contract.
At this point, Mallory's cross-domain withdrawal message in the old OVM_L2ToL1MessagePasser has been migrated to the storage of the new L2ToL1MessagePasser contract.
The L1's L1CrossDomainMessenger gets upgraded to the new implementation. Note that the state of successfulMessages mapping still exist after the upgrade.
Steps to be done after the migration
Mallory calls the OptimismPortal.proveWithdrawalTransaction to replay the cross-domain withdrawal message that bridges 100 ETH. It is possible for him to prove the withdrawal transaction because it exists in the storage of the L2ToL1MessagePasser contract.
After the waiting period (7-day), Mallory calls the OptimismPortal.finalizeWithdrawalTransaction function, which will in turn call the L1CrossDomainMessenger.relayMessage function.
As the withdrawal transaction has been converted to the new bedrock-style withdrawal, the nonce has been changed to a versioned nonce, which consists of the (version + nonce). The new nonce is as follows:
0x0000000000000000000000000000000000000000000000000000000000000999
0xVVNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN
V = Version Info, N = Nonce Counter
xDomainCalldataHash = keccak256(
abi.encodeWithSignature(
"relayMessage(address,address,bytes,uint256)",
_target,
_sender,
_message,
_messageNonce <=== this is "0000000000000000000000000000000000000000000000000000000000000999"
)
)
This will generate a new hash for the migrated legacy withdrawal and bypass the validation check at Line 275 below, and cause the message to be relayed again.
File: CrossDomainMessenger.sol
270: // If the message is version 0, then it's a migrated legacy withdrawal. We therefore need
271: // to check that the legacy version of the message has not already been relayed.
272: if (version == 0) {
273: bytes32 oldHash = Hashing.hashCrossDomainMessageV0(_target, _sender, _message, _nonce);
274: require(
275: successfulMessages[oldHash] == false,
276: "CrossDomainMessenger: legacy withdrawal already relayed"
277: );
278: }
Impact
High. Loss of assets. This allows anyone who has bridged assets from L2 to L1 before the migration to bridge the assets again for the second time after the migration, effectively resulting in a double-spend bug.
Update the implementation to compute the old hash of the legacy message by using the legacy-style nonce (without versioning) that contains only the Nonce counter.
function relayMessage(
uint256 _nonce,
address _sender,
address _target,
uint256 _value,
uint256 _minGasLimit,
bytes calldata _message
) external payable nonReentrant whenNotPaused {
+ (uint240 legacyNonce, uint16 version) = Encoding.decodeVersionedNonce(_nonce);
- (, uint16 version) = Encoding.decodeVersionedNonce(_nonce);
require(
version < 2,
"CrossDomainMessenger: only version 0 or 1 messages are supported at this time"
);
// If the message is version 0, then it's a migrated legacy withdrawal. We therefore need
// to check that the legacy version of the message has not already been relayed.
if (version == 0) {
+ bytes32 oldHash = Hashing.hashCrossDomainMessageV0(_target, _sender, _message, legacyNonce);
- bytes32 oldHash = Hashing.hashCrossDomainMessageV0(_target, _sender, _message, _nonce);
require(
successfulMessages[oldHash] == false,
"CrossDomainMessenger: legacy withdrawal already relayed"
);
}
xiaoming90
high
Legacy Message Can Be Replayed After Migration Leading To Double-Spend Bug
Summary
There is a bug while computing the old hash of the legacy message that allows legacy messages to be replayed. This allows anyone who has bridged assets from L2 to L1 before the migration to bridge the assets again for the second time after the migration, effectively resulting in a double-spend bug.
Vulnerability Detail
The following is the new
relayMessagefunction in Bedrock'sL1CrossDomainMessengercontract.https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol#L257
Note that the
_noncepassed into the newrelayMessagefunction is a versioned Nonce that consists of two components (versioning and nonce). The format of the versioned Nonce is as follows:During the migration, the LegacyWithdrawal is converted into the new bedrock-style Withdrawal.
The legacy-style nonce in the LegacyWithdrawal consists only of an Integer Nonce counter without any versioning information. During the conversion, it was replaced with a versioned Nonce. This is done for both the inner
relayMessagepayload and outer withdrawal payload. Refer to Lines 69, 74, and 89 belowhttps://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/op-chain-ops/crossdomain/migrate.go#L54
The following is the new
relayMessagefunction in the Bedrock'sL1CrossDomainMessengercontract. Line 272-278 performs validation to ensure that the legacy version of the message has not already been relayed. It does this by performing the following actions:1) Decoding the
_nonceprovided to determine the version. 2) If the nonce's version is zero, this means that it is a legacy withdrawal. Compute the old hash by callingHashing.hashCrossDomainMessageV0function with the_target,_sender,_messageand_nonce3) Verify if the old hash of the legacy message is set totruein thesuccessfulMessagesmapping. If it is true, this means that the message has already been relayed once before the migration, and the transaction will be reverted to prevent the legacy message from replaying in Bedrock.The issue here is that the
Hashing.hashCrossDomainMessageV0expects the legacy-style nonce (without versioning) that contains only the Integer Nonce counter. However, the code passes a versioned nonce into theHashing.hashCrossDomainMessageV0function to compute the old hash of the legacy message. As a result, the hash computed will not match the actual old hash of the legacy message, andsuccessfulMessages[oldHash]will returnfalse. This allows legacy message that has been relayed before migration to be relayed again in Bedrock after the migration.https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol#L273
Proof-of-Concept
Assume that Mallory is the attacker attempting to exploit this issue, and Mallory's wallet address in both L1 and L2 is
0x536fbBaE279fd77FAe3E29b410f7B605bf45BC8b.Steps to be done before Bedrock migration:
1) Mallory bridges 100 ETH from L2 to L1 by calling the
L2StandardBridge.withdrawfunction2) The
L2StandardBridgewill call theL2CrossDomainMessengerto send a cross-domain withdrawal message toL1CrossDomainMessenger3) The
L2CrossDomainMessengerwill call theOVM_L2ToL1MessagePasser.passMessageToL1to store the hash of the cross-domain withdrawal message in the storage ofsentMessagesmapping. ThesentMessagesmapping on theOVM_L2ToL1MessagePasser.passMessageToL1will be used by theL1CrossDomainMessenger._verifyStorageProof()function at L1 to determine if a message has been sent on L2.4) Once the cross-domain message gets relayed to L1, the
L1CrossDomainMessenger.relayMessagefunction will be triggered. TheL1CrossDomainMessengerwill call theL1StandardBridge.finalizeETHWithdrawalto transfer 100 ETH to Mallory's address in L1.5) After the transfer is done, the
successfulMessages[xDomainCalldataHash]is set totrueto prevent the message from being replayed. Note that the key of thesuccessfulMessagesis derived from thexDomainCalldataHash. ThexDomainCalldataHashis generated via the following algorithm:Assume that the
_messageNoncehere is999. Thus, the999value is one of the inputs used to compute the hash. Assume that the generatedxDomainCalldataHashisc89efdaa54c0f20c7adf612882df0950f5a951637e0307cdcb4c672f298b8bc6.As such,
successfulMessages[c89efdaa54c0f20c7adf612882df0950f5a951637e0307cdcb4c672f298b8bc6] = true.During the migration:
OVM_L2ToL1MessagePasserwill be converted to new bedrock-style withdrawals and stored in thesentMessagesmapping of the newL2ToL1MessagePassercontract.OVM_L2ToL1MessagePasserhas been migrated to the storage of the newL2ToL1MessagePassercontract.L1CrossDomainMessengergets upgraded to the new implementation. Note that the state ofsuccessfulMessagesmapping still exist after the upgrade.Steps to be done after the migration
Mallory calls the
OptimismPortal.proveWithdrawalTransactionto replay the cross-domain withdrawal message that bridges 100 ETH. It is possible for him to prove the withdrawal transaction because it exists in the storage of theL2ToL1MessagePassercontract.After the waiting period (7-day), Mallory calls the
OptimismPortal.finalizeWithdrawalTransactionfunction, which will in turn call theL1CrossDomainMessenger.relayMessagefunction.As the withdrawal transaction has been converted to the new bedrock-style withdrawal, the nonce has been changed to a versioned nonce, which consists of the (version + nonce). The new nonce is as follows:
This will generate a new hash for the migrated legacy withdrawal and bypass the validation check at Line 275 below, and cause the message to be relayed again.
https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol#L270
Impact
High. Loss of assets. This allows anyone who has bridged assets from L2 to L1 before the migration to bridge the assets again for the second time after the migration, effectively resulting in a double-spend bug.
Code Snippet
https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol#L273
Tool used
Manual Review
Recommendation
Update the implementation to compute the old hash of the legacy message by using the legacy-style nonce (without versioning) that contains only the Nonce counter.