Skip to content

Commit

Permalink
fix(protocol): oz - use excessivelySafeCall instadd of to.call(...) (
Browse files Browse the repository at this point in the history
  • Loading branch information
dantaik authored Feb 28, 2024
1 parent 6d7d3af commit 8d79dde
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 8 deletions.
2 changes: 1 addition & 1 deletion packages/protocol/contracts/L2/Lib1559Math.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

pragma solidity 0.8.24;

import "../thirdparty/LibFixedPointMath.sol";
import "../thirdparty/solmate/LibFixedPointMath.sol";

/// @title Lib1559Math
/// @dev Implementation of e^(x) based bonding curve for EIP-1559
Expand Down
9 changes: 8 additions & 1 deletion packages/protocol/contracts/bridge/Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import "@openzeppelin/contracts/utils/Address.sol";
import "../common/EssentialContract.sol";
import "../libs/LibAddress.sol";
import "../signal/ISignalService.sol";
import "../thirdparty/nomad-xyz/ExcessivelySafeCall.sol";
import "./IBridge.sol";

/// @title Bridge
Expand Down Expand Up @@ -540,7 +541,13 @@ contract Bridge is EssentialContract, IBridge {
) {
success = false;
} else {
(success,) = message.to.call{ value: message.value, gas: gasLimit }(message.data);
(success,) = ExcessivelySafeCall.excessivelySafeCall(
message.to,
gasLimit,
message.value,
64, // return max 64 bytes
message.data
);
}

// Reset the context after the message call
Expand Down
14 changes: 9 additions & 5 deletions packages/protocol/contracts/libs/LibAddress.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ import "@openzeppelin/contracts/utils/Address.sol";
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import "@openzeppelin/contracts/utils/introspection/IERC165.sol";
import "@openzeppelin/contracts/interfaces/IERC1271.sol";

import "../thirdparty/nomad-xyz/ExcessivelySafeCall.sol";
/// @title LibAddress
/// @dev Provides utilities for address-related operations.

library LibAddress {
bytes4 private constant EIP1271_MAGICVALUE = 0x1626ba7e;

Expand All @@ -35,10 +36,13 @@ library LibAddress {
if (to == address(0)) revert ETH_TRANSFER_FAILED();

// Attempt to send Ether to the recipient address
// WARNING: call() functions do not have an upper gas cost limit, so
// it's important to note that it may not reliably execute as expected
// when invoked with untrusted addresses.
(bool success,) = payable(to).call{ value: amount, gas: gasLimit }("");
(bool success,) = ExcessivelySafeCall.excessivelySafeCall(
to,
gasLimit,
amount,
64, // return max 64 bytes
""
);

// Ensure the transfer was successful
if (!success) revert ETH_TRANSFER_FAILED();
Expand Down
4 changes: 4 additions & 0 deletions packages/protocol/contracts/thirdparty/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# ABOUT THIRDPARTY CODE

- /optimism: code copied from `packages/contracts-bedrock/src/libraries` in https://github.com/ethereum-optimism/optimism/releases/tag/op-batcher%2Fv1.4.3 as-is with only solidity pragma changed.

- /solmate: code copied from https://github.com/transmissions11/solmate/blob/v7/src/utils/FixedPointMathLib.sol as-is with only solidity pragma changed.

- /nomad-xyz: code copied from https://github.com/nomad-xyz/ExcessivelySafeCall/blob/main/src/ExcessivelySafeCall.sol with unused coded removed and solidity pragma changed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
// https://github.com/nomad-xyz/ExcessivelySafeCall/blob/main/src/ExcessivelySafeCall.sol
pragma solidity 0.8.24;

library ExcessivelySafeCall {
uint256 constant LOW_28_MASK =
0x00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff;

/// @notice Use when you _really_ really _really_ don't trust the called
/// contract. This prevents the called contract from causing reversion of
/// the caller in as many ways as we can.
/// @dev The main difference between this and a solidity low-level call is
/// that we limit the number of bytes that the callee can cause to be
/// copied to caller memory. This prevents stupid things like malicious
/// contracts returning 10,000,000 bytes causing a local OOG when copying
/// to memory.
/// @param _target The address to call
/// @param _gas The amount of gas to forward to the remote contract
/// @param _value The value in wei to send to the remote contract
/// @param _maxCopy The maximum number of bytes of returndata to copy
/// to memory.
/// @param _calldata The data to send to the remote contract
/// @return success and returndata, as `.call()`. Returndata is capped to
/// `_maxCopy` bytes.
function excessivelySafeCall(
address _target,
uint256 _gas,
uint256 _value,
uint16 _maxCopy,
bytes memory _calldata
)
internal
returns (bool, bytes memory)
{
// set up for assembly call
uint256 _toCopy;
bool _success;
bytes memory _returnData = new bytes(_maxCopy);
// dispatch message to recipient
// by assembly calling "handle" function
// we call via assembly to avoid memcopying a very large returndata
// returned by a malicious contract
assembly {
_success :=
call(
_gas, // gas
_target, // recipient
_value, // ether value
add(_calldata, 0x20), // inloc
mload(_calldata), // inlen
0, // outloc
0 // outlen
)
// limit our copy to 256 bytes
_toCopy := returndatasize()
if gt(_toCopy, _maxCopy) { _toCopy := _maxCopy }
// Store the length of the copied bytes
mstore(_returnData, _toCopy)
// copy the bytes from returndata[0:_toCopy]
returndatacopy(add(_returnData, 0x20), 0, _toCopy)
}
return (_success, _returnData);
}
}
2 changes: 1 addition & 1 deletion packages/protocol/test/TaikoTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "forge-std/src/console2.sol";
import "@openzeppelin/contracts/utils/math/SafeCast.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";

import "../contracts/thirdparty/LibFixedPointMath.sol";
import "../contracts/thirdparty/solmate/LibFixedPointMath.sol";

import "../contracts/bridge/Bridge.sol";
import "../contracts/signal/SignalService.sol";
Expand Down

0 comments on commit 8d79dde

Please sign in to comment.