Commit 2dfcdf93 authored by clabby's avatar clabby

Fix portal gas limit dos v2

parent c88e8607
...@@ -403,6 +403,8 @@ ResourceMetering_Test:test_meter_updateTenEmptyBlocks_succeeds() (gas: 21161) ...@@ -403,6 +403,8 @@ ResourceMetering_Test:test_meter_updateTenEmptyBlocks_succeeds() (gas: 21161)
ResourceMetering_Test:test_meter_updateTwoEmptyBlocks_succeeds() (gas: 21117) ResourceMetering_Test:test_meter_updateTwoEmptyBlocks_succeeds() (gas: 21117)
ResourceMetering_Test:test_meter_useMax_succeeds() (gas: 8017416) ResourceMetering_Test:test_meter_useMax_succeeds() (gas: 8017416)
ResourceMetering_Test:test_meter_useMoreThanMax_reverts() (gas: 16045) ResourceMetering_Test:test_meter_useMoreThanMax_reverts() (gas: 16045)
SafeCall_call_Test:test_callWithMinGas_noLeakageHigh_succeeds() (gas: 388610034)
SafeCall_call_Test:test_callWithMinGas_noLeakageLow_succeeds() (gas: 142093281)
Semver_Test:test_behindProxy_succeeds() (gas: 506748) Semver_Test:test_behindProxy_succeeds() (gas: 506748)
Semver_Test:test_version_succeeds() (gas: 9418) Semver_Test:test_version_succeeds() (gas: 9418)
SequencerFeeVault_Test:test_constructor_succeeds() (gas: 5526) SequencerFeeVault_Test:test_constructor_succeeds() (gas: 5526)
...@@ -416,4 +418,3 @@ SystemConfig_Setters_TestFail:test_setGasConfig_notOwner_reverts() (gas: 10555) ...@@ -416,4 +418,3 @@ SystemConfig_Setters_TestFail:test_setGasConfig_notOwner_reverts() (gas: 10555)
SystemConfig_Setters_TestFail:test_setGasLimit_notOwner_reverts() (gas: 10658) SystemConfig_Setters_TestFail:test_setGasLimit_notOwner_reverts() (gas: 10658)
SystemConfig_Setters_TestFail:test_setUnsafeBlockSigner_notOwner_reverts() (gas: 10660) SystemConfig_Setters_TestFail:test_setUnsafeBlockSigner_notOwner_reverts() (gas: 10660)
TransferOnionTest:test_constructor_succeeds() (gas: 564855) TransferOnionTest:test_constructor_succeeds() (gas: 564855)
TransferOnionTest:test_unwrap_succeeds() (gas: 724958)
\ No newline at end of file
...@@ -363,26 +363,16 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -363,26 +363,16 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
// Mark the withdrawal as finalized so it can't be replayed. // Mark the withdrawal as finalized so it can't be replayed.
finalizedWithdrawals[withdrawalHash] = true; finalizedWithdrawals[withdrawalHash] = true;
// We want to maintain the property that the amount of gas supplied to the call to the
// target contract is at least the gas limit specified by the user. We can do this by
// enforcing that, at this point in time, we still have gaslimit + buffer gas available.
require(
gasleft() >= _tx.gasLimit + FINALIZE_GAS_BUFFER,
"OptimismPortal: insufficient gas to finalize withdrawal"
);
// Set the l2Sender so contracts know who triggered this withdrawal on L2. // Set the l2Sender so contracts know who triggered this withdrawal on L2.
l2Sender = _tx.sender; l2Sender = _tx.sender;
// Trigger the call to the target contract. We use SafeCall because we don't // Trigger the call to the target contract. We use SafeCall because we don't
// care about the returndata and we don't want target contracts to be able to force this // care about the returndata and we don't want target contracts to be able to force this
// call to run out of gas via a returndata bomb. // call to run out of gas via a returndata bomb. We also want to maintain the property
bool success = SafeCall.call( // that the amount of gas supplied to the call to the target contract is at least the gas
_tx.target, // limit specified by the user. If there is not enough gas in the callframe to accomplish
gasleft() - FINALIZE_GAS_BUFFER, // this, `callWithMinGas` will revert.
_tx.value, bool success = SafeCall.callWithMinGas(_tx.target, _tx.gasLimit, _tx.value, _tx.data);
_tx.data
);
// Reset the l2Sender back to the default value. // Reset the l2Sender back to the default value.
l2Sender = Constants.DEFAULT_L2_SENDER; l2Sender = Constants.DEFAULT_L2_SENDER;
......
...@@ -34,4 +34,70 @@ library SafeCall { ...@@ -34,4 +34,70 @@ library SafeCall {
} }
return _success; return _success;
} }
/**
* @notice Perform a low level call without copying any returndata. This function
* will revert if the call cannot be performed with the specified minimum
* gas.
*
* @param _target Address to call
* @param _minGas The minimum amount of gas that may be passed to the call
* @param _value Amount of value to pass to the call
* @param _calldata Calldata to pass to the call
*/
function callWithMinGas(
address _target,
uint256 _minGas,
uint256 _value,
bytes memory _calldata
) internal returns (bool) {
bool _success;
assembly {
// Assertion: gasleft() >= ((_minGas + 200) * 64) / 63
//
// Because EIP-150 ensures that, at max, 64/64ths of the remaining gas in the call
// frame may be passed to a subcontext, we need to ensure that the gas will not be
// truncated to hold the invariant. In addition, exactly 51 gas is consumed between
// the below `GAS` opcode and the `CALL` opcode, so it is factored in with some extra
// room for error.
if lt(gas(), div(shl(0x06, add(_minGas, 0xC8)), 0x3F)) {
// Store the "Error(string)" selector in scratch space.
mstore(0x00, 0x08c379a0)
// Store the pointer to the string length in scratch space.
mstore(0x20, 0x20)
// Store the string.
//
// SAFETY:
// - We pad the beginning of the string with two zero bytes as well as the
// length (24) to ensure that we override the free memory pointer at offset
// 0x40. This is necessary because the free memory pointer is likely to
// be greater than 1 byte when this function is called, but it is incredibly
// unlikely that it will be greater than 3 bytes. As for the data within
// 0x60, it is ensured that it is 0 due to 0x60 being the zero offset.
// - It's fine to clobber the free memory pointer, we're reverting.
mstore(0x58, 0x0000185361666543616c6c3a204e6f7420656e6f75676820676173)
// Revert with 'Error("SafeCall: Not enough gas")'
revert(0x1c, 0x64)
}
// The call will be supplied at least (((_minGas + 200) * 64) / 63) - 49 gas due to the
// above assertion. This ensures that, in all circumstances, the call will
// receive at least the minimum amount of gas specified.
// We can prove this property by solving the inequalities:
// ((((_minGas + 200) * 64) / 63) - 49) >= _minGas
// ((((_minGas + 200) * 64) / 63) - 51) * (63 / 64) >= _minGas
// Both inequalities hold true for all possible values of `_minGas`.
_success := call(
gas(), // gas
_target, // recipient
_value, // ether value
add(_calldata, 0x20), // inloc
mload(_calldata), // inlen
0x00, // outloc
0x00 // outlen
)
}
return _success;
}
} }
...@@ -39,4 +39,109 @@ contract SafeCall_call_Test is CommonTest { ...@@ -39,4 +39,109 @@ contract SafeCall_call_Test is CommonTest {
assertEq(to.balance, value, "to balance received"); assertEq(to.balance, value, "to balance received");
assertEq(from.balance, 0, "from balance not drained"); assertEq(from.balance, 0, "from balance not drained");
} }
function testFuzz_callWithMinGas_hasEnough_succeeds(
address from,
address to,
uint64 minGas,
uint64 value,
bytes memory data
) external {
vm.assume(from.balance == 0);
vm.assume(to.balance == 0);
// no precompiles
vm.assume(uint160(to) > 10);
// don't call the vm
vm.assume(to != address(vm));
vm.assume(from != address(vm));
// don't call the console
vm.assume(to != address(0x000000000000000000636F6e736F6c652e6c6f67));
// don't call the create2 deployer
vm.assume(to != address(0x4e59b44847b379578588920cA78FbF26c0B4956C));
// don't send funds to self
vm.assume(from != to);
assertEq(from.balance, 0, "from balance is 0");
vm.deal(from, value);
assertEq(from.balance, value, "from balance not dealt");
// Bound minGas to [0, l1_block_gas_limit]
minGas = uint64(bound(minGas, 0, 30_000_000));
vm.expectCallMinGas(to, value, minGas, data);
vm.prank(from);
bool success = SafeCall.callWithMinGas(to, minGas, value, data);
assertEq(success, true, "call not successful");
assertEq(to.balance, value, "to balance received");
assertEq(from.balance, 0, "from balance not drained");
}
function test_callWithMinGas_noLeakageLow_succeeds() external {
SimpleSafeCaller caller = new SimpleSafeCaller();
for (uint64 i = 5000; i < 50000; i++) {
// 26,069 is the exact amount of gas required to make the safe call
// successfully.
if (i < 26069) {
assertFalse(caller.makeSafeCall(i, 25000));
} else {
vm.expectCallMinGas(
address(caller),
0,
25000,
abi.encodeWithSelector(caller.setA.selector, 1)
);
assertTrue(caller.makeSafeCall(i, 25000));
}
}
}
function test_callWithMinGas_noLeakageHigh_succeeds() external {
SimpleSafeCaller caller = new SimpleSafeCaller();
for (uint256 i = 15_200_000; i < 15_300_000; i++) {
// 15,238,767 is the exact amount of gas required to make the safe call
// successfully.
if (i < 15_238_767) {
assertFalse(caller.makeSafeCall(uint64(i), 15_000_000));
} else {
vm.expectCallMinGas(
address(caller),
0,
15_000_000,
abi.encodeWithSelector(caller.setA.selector, 1)
);
assertTrue(caller.makeSafeCall(uint64(i), 15_000_000));
}
}
}
}
contract SimpleSafeCaller {
uint256 public a;
function makeSafeCall(uint64 gas, uint64 minGas) external returns (bool) {
return
SafeCall.call(
address(this),
gas,
0,
abi.encodeWithSelector(this.makeSafeCallMinGas.selector, minGas)
);
}
function makeSafeCallMinGas(uint64 minGas) external returns (bool) {
return
SafeCall.callWithMinGas(
address(this),
minGas,
0,
abi.encodeWithSelector(this.setA.selector, 1)
);
}
function setA(uint256 _a) external {
a = _a;
}
} }
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
import { Test } from "forge-std/Test.sol";
import { StdUtils } from "forge-std/StdUtils.sol";
import { Vm } from "forge-std/Vm.sol";
import { SafeCall } from "../../libraries/SafeCall.sol";
contract SafeCall_Succeeds_Invariants is Test {
SafeCaller_Succeeds_Actor actor;
function setUp() public {
// Create a new safe caller actor.
actor = new SafeCaller_Succeeds_Actor(vm);
// Set the caller to this contract
targetSender(address(this));
// Target the safe caller actor.
targetContract(address(actor));
}
/**
* @custom:invariant `callWithMinGas` forwards at least `minGas` if the call succeeds.
*
* If the call to `SafeCall.callWithMinGas` succeeds, then the
* call must have received at *least* `minGas` gas. If there is not enough gas in
* the callframe to supply the minimum amount of gas to the call, it must revert.
*/
function invariant_callWithMinGas_alwaysForwardsMinGas_succeeds() public {
assertEq(actor.numFailed(), 0, "no failed calls allowed");
}
function performSafeCallMinGas(uint64 minGas) external {
SafeCall.callWithMinGas(address(0), minGas, 0, hex"");
}
}
contract SafeCall_Fails_Invariants is Test {
SafeCaller_Fails_Actor actor;
function setUp() public {
// Create a new safe caller actor.
actor = new SafeCaller_Fails_Actor(vm);
// Set the caller to this contract
targetSender(address(this));
// Target the safe caller actor.
targetContract(address(actor));
}
/**
* @custom:invariant `callWithMinGas` reverts if there is not enough gas to pass
* to the call.
*
* If there is not enough gas in the callframe to ensure that
* `SafeCall.callWithMinGas` will receive at least `minGas` gas, then the call
* must revert.
*/
function invariant_callWithMinGas_neverForwardsMinGas_reverts() public {
assertEq(actor.numSuccessful(), 0, "no successful calls allowed");
}
function performSafeCallMinGas(uint64 minGas) external {
SafeCall.callWithMinGas(address(0), minGas, 0, hex"");
}
}
contract SafeCaller_Succeeds_Actor is StdUtils {
Vm internal vm;
uint256 public numFailed;
constructor(Vm _vm) {
vm = _vm;
}
function performSafeCallMinGas(uint64 gas, uint64 minGas) external {
// Bound the minimum gas amount to [2500, type(uint48).max]
minGas = uint64(bound(minGas, 2500, type(uint48).max));
// Bound the gas passed to [(((minGas + 200) * 64) / 63) + 500, type(uint64).max]
gas = uint64(bound(gas, (((minGas + 200) * 64) / 63) + 500, type(uint64).max));
vm.expectCallMinGas(address(0x00), 0, minGas, hex"");
bool success = SafeCall.call(
msg.sender,
gas,
0,
abi.encodeWithSelector(
SafeCall_Succeeds_Invariants.performSafeCallMinGas.selector,
minGas
)
);
if (!success) numFailed++;
}
}
contract SafeCaller_Fails_Actor is StdUtils {
Vm internal vm;
uint256 public numSuccessful;
constructor(Vm _vm) {
vm = _vm;
}
function performSafeCallMinGas(uint64 gas, uint64 minGas) external {
// Bound the minimum gas amount to [2500, type(uint48).max]
minGas = uint64(bound(minGas, 2500, type(uint48).max));
// Bound the gas passed to [minGas, (((minGas + 200) * 64) / 63)]
gas = uint64(bound(gas, minGas, (((minGas + 200) * 64) / 63)));
vm.expectCallMinGas(address(0x00), 0, minGas, hex"");
bool success = SafeCall.call(
msg.sender,
gas,
0,
abi.encodeWithSelector(SafeCall_Fails_Invariants.performSafeCallMinGas.selector, minGas)
);
if (success) numSuccessful++;
}
}
...@@ -14,6 +14,7 @@ This directory contains documentation for all defined invariant tests within `co ...@@ -14,6 +14,7 @@ This directory contains documentation for all defined invariant tests within `co
- [L2OutputOracle](./L2OutputOracle.md) - [L2OutputOracle](./L2OutputOracle.md)
- [OptimismPortal](./OptimismPortal.md) - [OptimismPortal](./OptimismPortal.md)
- [ResourceMetering](./ResourceMetering.md) - [ResourceMetering](./ResourceMetering.md)
- [SafeCall](./SafeCall.md)
- [SystemConfig](./SystemConfig.md) - [SystemConfig](./SystemConfig.md)
<!-- END autoTOC --> <!-- END autoTOC -->
......
# `SafeCall` Invariants
## `callWithMinGas` forwards at least `minGas` if the call succeeds.
**Test:** [`SafeCall.t.sol#L30`](../contracts/test/invariants/SafeCall.t.sol#L30)
If the call to `SafeCall.callWithMinGas` succeeds, then the call must have received at *least* `minGas` gas. If there is not enough gas in the callframe to supply the minimum amount of gas to the call, it must revert.
## `callWithMinGas` reverts if there is not enough gas to pass to the call.
**Test:** [`SafeCall.t.sol#L61`](../contracts/test/invariants/SafeCall.t.sol#L61)
If there is not enough gas in the callframe to ensure that `SafeCall.callWithMinGas` will receive at least `minGas` gas, then the call must revert.
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