From 3bb8d4b006cf508b549bd0ac4dce24de0c0cecbb Mon Sep 17 00:00:00 2001 From: excaliborr Date: Thu, 25 Jul 2024 10:27:30 -0400 Subject: [PATCH] fix: eip-712 compliance --- src/contracts/L1OpUSDCBridgeAdapter.sol | 7 +-- src/contracts/L2OpUSDCBridgeAdapter.sol | 7 +-- .../universal/OpUSDCBridgeAdapter.sol | 32 ++++++++-- src/interfaces/IOpUSDCBridgeAdapter.sol | 15 +++++ test/unit/OpUSDCBridgeAdapter.t.sol | 9 ++- test/utils/Helpers.sol | 16 ++++- test/utils/SigUtils.sol | 61 +++++++++++++++++++ 7 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 test/utils/SigUtils.sol diff --git a/src/contracts/L1OpUSDCBridgeAdapter.sol b/src/contracts/L1OpUSDCBridgeAdapter.sol index 42a32802..5b1e42f6 100644 --- a/src/contracts/L1OpUSDCBridgeAdapter.sol +++ b/src/contracts/L1OpUSDCBridgeAdapter.sol @@ -249,11 +249,10 @@ contract L1OpUSDCBridgeAdapter is IL1OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { // Ensure the deadline has not passed if (block.timestamp > _deadline) revert IOpUSDCBridgeAdapter_MessageExpired(); - // Hash the message - bytes32 _messageHash = - keccak256(abi.encode(address(this), block.chainid, _to, _amount, _deadline, _minGasLimit, _nonce)); + BridgeMessage memory _message = + BridgeMessage({to: _to, amount: _amount, deadline: _deadline, nonce: _nonce, minGasLimit: _minGasLimit}); - _checkSignature(_signer, _messageHash, _signature); + _checkSignature(_signer, _hashMessageStruct(_message), _signature); // Mark the nonce as used userNonces[_signer][_nonce] = true; diff --git a/src/contracts/L2OpUSDCBridgeAdapter.sol b/src/contracts/L2OpUSDCBridgeAdapter.sol index db5ab3cf..f42ff027 100644 --- a/src/contracts/L2OpUSDCBridgeAdapter.sol +++ b/src/contracts/L2OpUSDCBridgeAdapter.sol @@ -193,11 +193,10 @@ contract L2OpUSDCBridgeAdapter is IL2OpUSDCBridgeAdapter, OpUSDCBridgeAdapter { // Ensure the deadline has not passed if (block.timestamp > _deadline) revert IOpUSDCBridgeAdapter_MessageExpired(); - // Hash the message - bytes32 _messageHash = - keccak256(abi.encode(address(this), block.chainid, _to, _amount, _deadline, _minGasLimit, _nonce)); + BridgeMessage memory _message = + BridgeMessage({to: _to, amount: _amount, deadline: _deadline, nonce: _nonce, minGasLimit: _minGasLimit}); - _checkSignature(_signer, _messageHash, _signature); + _checkSignature(_signer, _hashMessageStruct(_message), _signature); // Mark the nonce as used userNonces[_signer][_nonce] = true; diff --git a/src/contracts/universal/OpUSDCBridgeAdapter.sol b/src/contracts/universal/OpUSDCBridgeAdapter.sol index 7c10fc77..ce53ca3d 100644 --- a/src/contracts/universal/OpUSDCBridgeAdapter.sol +++ b/src/contracts/universal/OpUSDCBridgeAdapter.sol @@ -2,14 +2,20 @@ pragma solidity 0.8.25; import {Ownable} from '@openzeppelin/contracts/access/Ownable.sol'; + +import {EIP712} from '@openzeppelin/contracts/utils/cryptography/EIP712.sol'; import {MessageHashUtils} from '@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol'; import {SignatureChecker} from '@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol'; import {IOpUSDCBridgeAdapter} from 'interfaces/IOpUSDCBridgeAdapter.sol'; -abstract contract OpUSDCBridgeAdapter is IOpUSDCBridgeAdapter, Ownable { +abstract contract OpUSDCBridgeAdapter is IOpUSDCBridgeAdapter, Ownable, EIP712 { using MessageHashUtils for bytes32; using SignatureChecker for address; + /// @notice The typehash for the bridge message + bytes32 public constant BRIDGE_MESSAGE_TYPEHASH = + keccak256('BridgeMessage(address to,uint256 amount,uint256 deadline,uint256 nonce,uint32 minGasLimit)'); + /// @inheritdoc IOpUSDCBridgeAdapter address public immutable USDC; @@ -32,8 +38,13 @@ abstract contract OpUSDCBridgeAdapter is IOpUSDCBridgeAdapter, Ownable { * @param _linkedAdapter The address of the linked adapter * @param _owner The address of the owner of the contract */ - // solhint-disable-next-line no-unused-vars - constructor(address _usdc, address _messenger, address _linkedAdapter, address _owner) Ownable(_owner) { + constructor( + address _usdc, + address _messenger, + address _linkedAdapter, + // solhint-disable-next-line no-unused-vars + address _owner + ) Ownable(_owner) EIP712('OpUSDCBridgeAdapter', '1.0.0') { USDC = _usdc; MESSENGER = _messenger; LINKED_ADAPTER = _linkedAdapter; @@ -100,8 +111,21 @@ abstract contract OpUSDCBridgeAdapter is IOpUSDCBridgeAdapter, Ownable { * @param _signature the signature of the message */ function _checkSignature(address _signer, bytes32 _messageHash, bytes memory _signature) internal view { - _messageHash = _messageHash.toEthSignedMessageHash(); + _messageHash = _hashTypedDataV4(_messageHash); if (!_signer.isValidSignatureNow(_messageHash, _signature)) revert IOpUSDCBridgeAdapter_InvalidSignature(); } + + /** + * @notice Hashes the bridge message struct + * @param _message The bridge message struct to hash + * @return _hash The hash of the bridge message struct + */ + function _hashMessageStruct(BridgeMessage memory _message) internal pure returns (bytes32 _hash) { + _hash = keccak256( + abi.encode( + BRIDGE_MESSAGE_TYPEHASH, _message.to, _message.amount, _message.deadline, _message.nonce, _message.minGasLimit + ) + ); + } } diff --git a/src/interfaces/IOpUSDCBridgeAdapter.sol b/src/interfaces/IOpUSDCBridgeAdapter.sol index 44c0fc00..2b080f1d 100644 --- a/src/interfaces/IOpUSDCBridgeAdapter.sol +++ b/src/interfaces/IOpUSDCBridgeAdapter.sol @@ -2,6 +2,21 @@ pragma solidity 0.8.25; interface IOpUSDCBridgeAdapter { + /** + * @notice The struct to hold the data for a bridge message with signature + * @param to The target address on the destination chain + * @param amount The amount of tokens to send + * @param deadline The deadline for the message to be executed + * @param nonce The nonce of the user + * @param minGasLimit The minimum gas limit for the message to be executed + */ + struct BridgeMessage { + address to; + uint256 amount; + uint256 deadline; + uint256 nonce; + uint32 minGasLimit; + } /*/////////////////////////////////////////////////////////////// EVENTS ///////////////////////////////////////////////////////////////*/ diff --git a/test/unit/OpUSDCBridgeAdapter.t.sol b/test/unit/OpUSDCBridgeAdapter.t.sol index 2b3d8f33..5a56dc68 100644 --- a/test/unit/OpUSDCBridgeAdapter.t.sol +++ b/test/unit/OpUSDCBridgeAdapter.t.sol @@ -5,6 +5,7 @@ import {MessageHashUtils} from '@openzeppelin/contracts/utils/cryptography/Messa import {OpUSDCBridgeAdapter} from 'contracts/universal/OpUSDCBridgeAdapter.sol'; import {Test} from 'forge-std/Test.sol'; import {IOpUSDCBridgeAdapter} from 'interfaces/IOpUSDCBridgeAdapter.sol'; +import {SigUtils} from 'test/utils/SigUtils.sol'; contract ForTestOpUSDCBridgeAdapter is OpUSDCBridgeAdapter { constructor( @@ -109,10 +110,12 @@ contract OpUSDCBridgeAdapter_Unit_CheckSignature is Base { /** * @notice Check that the signature is valid */ - function test_validSignature(bytes memory _message) public { + function test_validSignature(IOpUSDCBridgeAdapter.BridgeMessage memory _message) public { + SigUtils _sigUtils = new SigUtils(address(adapter)); + vm.startPrank(_signerAd); - bytes32 _hashedMessage = keccak256(abi.encodePacked(_message)); - bytes32 _digest = MessageHashUtils.toEthSignedMessageHash(_hashedMessage); + bytes32 _hashedMessage = _sigUtils.getBridgeMessageHash(_message); + bytes32 _digest = _sigUtils.getTypedBridgeMessageHash(_message); (uint8 v, bytes32 r, bytes32 s) = vm.sign(_signerPk, _digest); bytes memory _signature = abi.encodePacked(r, s, v); vm.stopPrank(); diff --git a/test/utils/Helpers.sol b/test/utils/Helpers.sol index 8d387084..b1c10499 100644 --- a/test/utils/Helpers.sol +++ b/test/utils/Helpers.sol @@ -3,6 +3,9 @@ pragma solidity ^0.8.25; import {MessageHashUtils} from '@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol'; import {Test} from 'forge-std/Test.sol'; +import {IOpUSDCBridgeAdapter} from 'interfaces/IOpUSDCBridgeAdapter.sol'; + +import {SigUtils} from 'test/utils/SigUtils.sol'; contract Helpers is Test { using MessageHashUtils for bytes32; @@ -31,9 +34,18 @@ contract Helpers is Test { uint256 _signerPk, address _adapter ) internal returns (bytes memory _signature) { + IOpUSDCBridgeAdapter.BridgeMessage memory _message = IOpUSDCBridgeAdapter.BridgeMessage({ + to: _to, + amount: _amount, + deadline: _deadline, + nonce: _nonce, + minGasLimit: uint32(_minGasLimit) + }); + + SigUtils _sigUtils = new SigUtils(_adapter); + vm.startPrank(_signerAd); - bytes32 _digest = keccak256(abi.encode(_adapter, block.chainid, _to, _amount, _deadline, _minGasLimit, _nonce)) - .toEthSignedMessageHash(); + bytes32 _digest = SigUtils(_sigUtils).getTypedBridgeMessageHash(_message); (uint8 v, bytes32 r, bytes32 s) = vm.sign(_signerPk, _digest); _signature = abi.encodePacked(r, s, v); vm.stopPrank(); diff --git a/test/utils/SigUtils.sol b/test/utils/SigUtils.sol new file mode 100644 index 00000000..4a4956d6 --- /dev/null +++ b/test/utils/SigUtils.sol @@ -0,0 +1,61 @@ +pragma solidity 0.8.25; + +import {ShortString, ShortStrings} from '@openzeppelin/contracts/utils/ShortStrings.sol'; +import {IOpUSDCBridgeAdapter} from 'interfaces/IOpUSDCBridgeAdapter.sol'; + +contract SigUtils { + using ShortStrings for *; + + bytes32 private constant _TYPE_HASH = + keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'); + bytes32 public constant BRIDGE_MESSAGE_TYPEHASH = + keccak256('BridgeMessage(address to,uint256 amount,uint256 deadline,uint256 nonce,uint32 minGasLimit)'); + + bytes32 internal immutable _DOMAIN_SEPARATOR; + + bytes32 private immutable _HASHED_NAME; + bytes32 private immutable _HASHED_VERSION; + + ShortString private immutable _NAME; + ShortString private immutable _VERSION; + string private _nameFallback; + string private _versionFallback; + + constructor(address _adapter) { + string memory _name = 'OpUSDCBridgeAdapter'; + string memory _version = '1.0.0'; + _NAME = _name.toShortStringWithFallback(_nameFallback); + _VERSION = _version.toShortStringWithFallback(_versionFallback); + + _HASHED_NAME = keccak256(bytes(_name)); + _HASHED_VERSION = keccak256(bytes(_version)); + + _DOMAIN_SEPARATOR = keccak256(abi.encode(_TYPE_HASH, _HASHED_NAME, _HASHED_VERSION, block.chainid, _adapter)); + } + + /** + * @notice Hashes the bridge message struct + * @param _message The bridge message struct to hash + * @return _hash The hash of the bridge message struct + */ + function getBridgeMessageHash(IOpUSDCBridgeAdapter.BridgeMessage memory _message) public view returns (bytes32 _hash) { + _hash = keccak256( + abi.encode( + BRIDGE_MESSAGE_TYPEHASH, _message.to, _message.amount, _message.deadline, _message.nonce, _message.minGasLimit + ) + ); + } + + /** + * @notice Hashes the bridge message struct and returns the EIP712 hash + * @param _message The bridge message struct to hash + * @return _hash The hash of the bridge message struct + */ + function getTypedBridgeMessageHash(IOpUSDCBridgeAdapter.BridgeMessage memory _message) + public + view + returns (bytes32 _hash) + { + _hash = keccak256(abi.encodePacked('\x19\x01', _DOMAIN_SEPARATOR, getBridgeMessageHash(_message))); + } +}