Commit 04884132 authored by Maurelian's avatar Maurelian Committed by GitHub

bedrock: fix OZ-H-O2 Token address ordering (#2778)

This change addresses the H-O2 finding in the OZ audit extension.
Co-authored-by: default avatarMatthew Slipper <me@matthewslipper.com>
Co-authored-by: default avatarmergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
parent 6c9a87a9
---
'@eth-optimism/contracts-bedrock': minor
---
Corrects the ordering of token addresses when a finalizeBridgeERC20 call fails
......@@ -71,11 +71,12 @@ L2OutputOracleTest:test_deleteL2Output() (gas: 64338)
L2OutputOracleTest:test_getL2Output() (gas: 74601)
L2OutputOracleTest:test_latestBlockTimestamp() (gas: 68377)
L2OutputOracleTest:test_nextTimestamp() (gas: 9236)
L2StandardBridge_Test:test_finalizeDeposit() (gas: 93191)
L2StandardBridge_Test:test_initialize() (gas: 14834)
L2StandardBridge_Test:test_receive() (gas: 136437)
L2StandardBridge_Test:test_withdraw() (gas: 352644)
L2StandardBridge_Test:test_withdrawTo() (gas: 353495)
L2StandardBridge_Test:test_finalizeDeposit() (gas: 93169)
L2StandardBridge_Test:test_finalizeDeposit_failsToCompleteOutboundTransfer() (gas: 140106)
L2StandardBridge_Test:test_initialize() (gas: 14812)
L2StandardBridge_Test:test_receive() (gas: 136415)
L2StandardBridge_Test:test_withdraw() (gas: 352626)
L2StandardBridge_Test:test_withdrawTo() (gas: 353477)
L2ToL1MessagePasserTest:test_burn() (gas: 112046)
L2ToL1MessagePasserTest:test_initiateWithdrawal_fromContract() (gas: 67890)
L2ToL1MessagePasserTest:test_initiateWithdrawal_fromEOA() (gas: 74851)
......
......@@ -260,6 +260,15 @@ contract Bridge_Initializer is Messenger_Initializer {
bytes _data
);
event ERC20BridgeFailed(
address indexed _localToken,
address indexed _remoteToken,
address indexed _from,
address _to,
uint256 _amount,
bytes _data
);
function setUp() public virtual override {
super.setUp();
......
......@@ -109,5 +109,48 @@ contract L2StandardBridge_Test is Bridge_Initializer {
hex""
);
}
// finalizeDeposit
// - only callable by l1TokenBridge
// - supported token pair emits DepositFinalized
// - invalid deposit emits DepositFailed
// - invalid deposit calls Withdrawer.initiateWithdrawal
function test_finalizeDeposit_failsToCompleteOutboundTransfer() external {
// TODO: events and calls
address invalidL2Token = address(0x1234);
vm.mockCall(
address(L2Bridge.messenger()),
abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
abi.encode(address(L2Bridge.otherBridge()))
);
vm.prank(address(L2Messenger));
vm.expectEmit(true, true, true, true);
emit ERC20BridgeInitiated(
invalidL2Token,
address(L1Token),
alice,
alice,
100,
hex""
);
vm.expectEmit(true, true, true, true);
emit ERC20BridgeFailed(
invalidL2Token,
address(L1Token),
alice,
alice,
100,
hex""
);
L2Bridge.finalizeDeposit(
address(L1Token),
invalidL2Token,
alice,
alice,
100,
hex""
);
}
}
......@@ -226,8 +226,8 @@ abstract contract StandardBridge {
// Something went wrong during the bridging process, return to sender.
// Can happen if a bridge UI specifies the wrong L2 token.
_initiateBridgeERC20Unchecked(
_remoteToken,
_localToken,
_remoteToken,
_from,
_to,
_amount,
......@@ -352,6 +352,9 @@ abstract contract StandardBridge {
address(otherBridge),
abi.encodeWithSelector(
this.finalizeBridgeERC20.selector,
// Because this call will be executed on the remote chain, we reverse the order of
// the remote and local token addresses relative to their order in the
// finalizeBridgeERC20 function.
_remoteToken,
_localToken,
_from,
......
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