diff --git a/contracts/access/access_control/_AccessControl.sol b/contracts/access/access_control/_AccessControl.sol index b21d14cc..0c1c5ac7 100644 --- a/contracts/access/access_control/_AccessControl.sol +++ b/contracts/access/access_control/_AccessControl.sol @@ -50,7 +50,7 @@ abstract contract _AccessControl is _IAccessControl, _Context { * @notice revert if sender does not have given role * @param role role to query */ - function _checkRole(bytes32 role) internal view virtual { + function _checkRole(bytes32 role) internal virtual { _checkRole(role, _msgSender()); } diff --git a/contracts/index.sol b/contracts/index.sol index dee26176..9f66d6f5 100644 --- a/contracts/index.sol +++ b/contracts/index.sol @@ -117,12 +117,16 @@ import './introspection/Introspectable.sol'; import './introspection/_IIntrospectable.sol'; import './introspection/_Introspectable.sol'; import './meta/Context.sol'; +import './meta/ECDSAMetaTransactionContext.sol'; import './meta/ForwardedMetaTransactionContext.sol'; import './meta/IContext.sol'; +import './meta/IECDSAMetaTransactionContext.sol'; import './meta/IForwardedMetaTransactionContext.sol'; import './meta/_Context.sol'; +import './meta/_ECDSAMetaTransactionContext.sol'; import './meta/_ForwardedMetaTransactionContext.sol'; import './meta/_IContext.sol'; +import './meta/_IECDSAMetaTransactionContext.sol'; import './meta/_IForwardedMetaTransactionContext.sol'; import './proxy/IProxy.sol'; import './proxy/Proxy.sol'; diff --git a/contracts/meta/ECDSAMetaTransactionContext.sol b/contracts/meta/ECDSAMetaTransactionContext.sol new file mode 100644 index 00000000..afd6e753 --- /dev/null +++ b/contracts/meta/ECDSAMetaTransactionContext.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.24; + +import { IERC5267 } from '../interfaces/IERC5267.sol'; +import { TransientReentrancyGuard } from '../access/reentrancy_guard/TransientReentrancyGuard.sol'; +import { _TransientReentrancyGuard } from '../access/reentrancy_guard/_TransientReentrancyGuard.sol'; +import { Context } from './Context.sol'; +import { _Context } from './_Context.sol'; +import { IECDSAMetaTransactionContext } from './IECDSAMetaTransactionContext.sol'; +import { _ECDSAMetaTransactionContext } from './_ECDSAMetaTransactionContext.sol'; + +abstract contract ECDSAMetaTransactionContext is + IECDSAMetaTransactionContext, + _ECDSAMetaTransactionContext, + Context, + TransientReentrancyGuard +{ + /** + * @inheritdoc IERC5267 + */ + function eip712Domain() + external + view + returns ( + bytes1 fields, + string memory name, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + uint256[] memory extensions + ) + { + return _eip712Domain(); + } + + function _msgSender() + internal + override(_Context, _ECDSAMetaTransactionContext) + returns (address msgSender) + { + msgSender = super._msgSender(); + } + + function _msgData() + internal + override(_Context, _ECDSAMetaTransactionContext) + returns (bytes calldata msgData) + { + msgData = super._msgData(); + } + + function _calldataSuffixLength() + internal + view + override(_Context, _ECDSAMetaTransactionContext) + returns (uint256 length) + { + length = super._calldataSuffixLength(); + } + + function _isReentrancyGuardLocked() + internal + view + virtual + override(_TransientReentrancyGuard, TransientReentrancyGuard) + returns (bool status) + { + status = super._isReentrancyGuardLocked(); + } + + function _lockReentrancyGuard() + internal + virtual + override(TransientReentrancyGuard, _ECDSAMetaTransactionContext) + { + super._lockReentrancyGuard(); + } + + function _unlockReentrancyGuard() + internal + virtual + override(_TransientReentrancyGuard, TransientReentrancyGuard) + { + super._unlockReentrancyGuard(); + } +} diff --git a/contracts/meta/ForwardedMetaTransactionContext.sol b/contracts/meta/ForwardedMetaTransactionContext.sol index 3b830b00..a06a673d 100644 --- a/contracts/meta/ForwardedMetaTransactionContext.sol +++ b/contracts/meta/ForwardedMetaTransactionContext.sol @@ -27,7 +27,6 @@ abstract contract ForwardedMetaTransactionContext is */ function _msgSender() internal - view virtual override(_Context, _ForwardedMetaTransactionContext) returns (address msgSender) @@ -40,7 +39,6 @@ abstract contract ForwardedMetaTransactionContext is */ function _msgData() internal - view virtual override(_Context, _ForwardedMetaTransactionContext) returns (bytes calldata msgData) diff --git a/contracts/meta/IECDSAMetaTransactionContext.sol b/contracts/meta/IECDSAMetaTransactionContext.sol new file mode 100644 index 00000000..ed9f74fe --- /dev/null +++ b/contracts/meta/IECDSAMetaTransactionContext.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.24; + +import { IERC5267 } from '../interfaces/IERC5267.sol'; +import { ITransientReentrancyGuard } from '../access/reentrancy_guard/ITransientReentrancyGuard.sol'; +import { IContext } from './IContext.sol'; +import { _IECDSAMetaTransactionContext } from './_IECDSAMetaTransactionContext.sol'; + +interface IECDSAMetaTransactionContext is + _IECDSAMetaTransactionContext, + IContext, + ITransientReentrancyGuard, + IERC5267 +{} diff --git a/contracts/meta/_Context.sol b/contracts/meta/_Context.sol index e344ded6..7fc4ed8e 100644 --- a/contracts/meta/_Context.sol +++ b/contracts/meta/_Context.sol @@ -13,7 +13,7 @@ abstract contract _Context is _IContext { * @dev if no Context extension is in use, msg.sender is returned as-is * @return msgSender account contextualized as message sender */ - function _msgSender() internal view virtual returns (address msgSender) { + function _msgSender() internal virtual returns (address msgSender) { msgSender = msg.sender; } @@ -22,7 +22,7 @@ abstract contract _Context is _IContext { * @dev if no Context extension is in use, msg.data is returned as-is * @return msgData message data with suffix removed, if applicable */ - function _msgData() internal view virtual returns (bytes calldata msgData) { + function _msgData() internal virtual returns (bytes calldata msgData) { msgData = msg.data; } diff --git a/contracts/meta/_ECDSAMetaTransactionContext.sol b/contracts/meta/_ECDSAMetaTransactionContext.sol new file mode 100644 index 00000000..8fca794c --- /dev/null +++ b/contracts/meta/_ECDSAMetaTransactionContext.sol @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.24; + +import { ECDSA } from '../cryptography/ECDSA.sol'; +import { EIP712 } from '../cryptography/EIP712.sol'; +import { sslot } from '../data/StorageSlot.sol'; +import { tslot } from '../data/TransientSlot.sol'; +import { _TransientReentrancyGuard } from '../access/reentrancy_guard/_TransientReentrancyGuard.sol'; +import { Bytes32 } from '../utils/Bytes32.sol'; +import { Bytes32Builder } from '../data/Bytes32Builder.sol'; +import { _Context } from './_Context.sol'; +import { _IECDSAMetaTransactionContext } from './_IECDSAMetaTransactionContext.sol'; + +abstract contract _ECDSAMetaTransactionContext is + _IECDSAMetaTransactionContext, + _Context, + _TransientReentrancyGuard +{ + using Bytes32 for bytes32; + using Bytes32Builder for Bytes32.Builder; + using ECDSA for bytes32; + + bytes32 internal constant EIP_712_TYPE_HASH = + keccak256( + 'ECDSAMetaTransaction(bytes msgData,uint256 msgValue,uint256 nonce)' + ); + + tslot private constant TRANSIENT_SLOT = + tslot.wrap( + keccak256(abi.encode(uint256(EIP_712_TYPE_HASH) - 1)) & + ~bytes32(uint256(0xff)) + ); + + function _eip712Domain() + internal + view + virtual + returns ( + bytes1 fields, + string memory name, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + uint256[] memory extensions + ) + { + return ( + EIP712.ERC5267_FIELDS_01100, + '', + '', + block.chainid, + address(this), + bytes32(0), + new uint256[](0) + ); + } + + /** + * @inheritdoc _Context + * @dev sender is read from the calldata context suffix + */ + function _msgSender() + internal + virtual + override + returns (address msgSender) + { + uint256 dataLength = msg.data.length; + uint256 suffixLength = _calldataSuffixLength(); + + // context is cached in transient storage, which is not cleared until the end of the transaction + // this enables the possibility of replay attacks within a single transaction which calls this contract multiple times + // therefore, all functions which use context must be nonReentrant, and the lock must be set before context is accessed + + if (dataLength >= suffixLength && _isReentrancyGuardLocked()) { + // calldata is long enough that it might have a suffix + // check transient storage to see if sender has been derived already + + // toAddress function strips the packed msgDataIndex data and returns a clean address + msgSender = TRANSIENT_SLOT.read().toBuilder().parseAddress(0); + + if (msgSender == address(0)) { + // no sender found in transient storage, so attempt to derive it from signature + + unchecked { + (msgSender, ) = _processCalldata(dataLength - suffixLength); + } + } + } else { + // calldata is too short for this to be a valid meta transaction + // return message sender as-is + msgSender = super._msgSender(); + } + } + + /** + * @inheritdoc _Context + */ + function _msgData() + internal + virtual + override + returns (bytes calldata msgData) + { + uint256 dataLength = msg.data.length; + uint256 suffixLength = _calldataSuffixLength(); + + // context is cached in transient storage, which is not cleared until the end of the transaction + // this enables the possibility of replay attacks within a single transaction which calls this contract multiple times + // therefore, all functions which use context must be nonReentrant, and the lock must be set before context is accessed + + if (dataLength >= suffixLength && _isReentrancyGuardLocked()) { + // calldata is long enough that it might have a suffix + // check transient storage to see if msgData split index has been derived already + + // unpack the msgDataIndex which is stored alongside msgSender + uint256 split = TRANSIENT_SLOT.read().toBuilder().parseUint96(160); + + if (split == 0) { + // no msgData split index found in transient storage, so attempt to derive it from signature + + unchecked { + (, split) = _processCalldata(dataLength - suffixLength); + } + } + + msgData = msg.data[:split]; + } else { + // calldata is too short for this to be a valid meta transaction + // return message data as-is + msgData = super._msgData(); + } + } + + /** + * @inheritdoc _Context + * @dev this Context extension defines a suffix with a length of 85 + */ + function _calldataSuffixLength() + internal + view + virtual + override + returns (uint256 length) + { + length = 85; + } + + function _processCalldata( + uint256 split + ) private returns (address msgSender, uint256 msgDataIndex) { + unchecked { + bytes calldata msgData = msg.data[:split]; + msgSender = address(bytes20(msg.data[split:split + 20])); + bytes calldata signature = msg.data[split + 20:]; + + // TODO: lookup and invalidate nonce + uint256 nonce = 1; + + // TODO: include msg.sender in hash to restrict forwarder? + bytes32 structHash = keccak256( + abi.encode( + EIP_712_TYPE_HASH, + keccak256(msgData), + msg.value, + nonce + ) + ); + + bytes32 recoverableHash = ECDSA.toEIP712RecoverableHash( + EIP712.calculateDomainSeparator_01100(), + structHash + ); + + // TODO: see what happens if split calldata v r s + address signer = recoverableHash.tryRecover(signature); + + if (signer == msgSender) { + msgDataIndex = split; + } else { + msgSender = super._msgSender(); + msgDataIndex = super._msgData().length; + } + } + + // it is necessary to store metadata in transient storage because + // subsequent derivation will fail due to nonce invalidation + + Bytes32.Builder memory builder; + + builder.pushAddress(msgSender); + builder.pushUint96(uint96(msgDataIndex)); + + TRANSIENT_SLOT.write(builder._data); + } + + /** + * @inheritdoc _TransientReentrancyGuard + * @dev clear the cached context to prevent replay attacks + */ + function _lockReentrancyGuard() internal virtual override { + TRANSIENT_SLOT.clear(); + super._lockReentrancyGuard(); + } +} diff --git a/contracts/meta/_ForwardedMetaTransactionContext.sol b/contracts/meta/_ForwardedMetaTransactionContext.sol index 409657f5..d126e067 100644 --- a/contracts/meta/_ForwardedMetaTransactionContext.sol +++ b/contracts/meta/_ForwardedMetaTransactionContext.sol @@ -19,7 +19,6 @@ abstract contract _ForwardedMetaTransactionContext is */ function _msgSender() internal - view virtual override returns (address msgSender) @@ -46,7 +45,6 @@ abstract contract _ForwardedMetaTransactionContext is */ function _msgData() internal - view virtual override returns (bytes calldata msgData) diff --git a/contracts/meta/_IECDSAMetaTransactionContext.sol b/contracts/meta/_IECDSAMetaTransactionContext.sol new file mode 100644 index 00000000..d96b5ab9 --- /dev/null +++ b/contracts/meta/_IECDSAMetaTransactionContext.sol @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.24; + +import { _IERC5267 } from '../interfaces/_IERC5267.sol'; +import { _ITransientReentrancyGuard } from '../access/reentrancy_guard/_ITransientReentrancyGuard.sol'; +import { _IContext } from './_IContext.sol'; + +interface _IECDSAMetaTransactionContext is + _IContext, + _ITransientReentrancyGuard, + _IERC5267 +{} diff --git a/lib/erc20_permit.ts b/lib/erc20_permit.ts index 08c6ac6f..d18defef 100644 --- a/lib/erc20_permit.ts +++ b/lib/erc20_permit.ts @@ -1,6 +1,6 @@ import { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/signers'; import { IERC2612, IERC5267 } from '@solidstate/typechain-types'; -import { Signature } from 'ethers'; +import { BytesLike, Signature } from 'ethers'; import { ethers } from 'hardhat'; interface Domain { @@ -99,9 +99,36 @@ const signERC2612Permit = async ( const signature = await owner.signTypedData(domain, types, values); - const permit = ethers.Signature.from(signature); + return ethers.Signature.from(signature); +}; + +// TODO: rename file or move to new file +const signECDSAMetaTransaction = async ( + instance: IERC5267, + signer: SignerWithAddress, + msgData: BytesLike, + msgValue: bigint, + nonce: bigint, +): Promise => { + const domain = await getDomain(instance); + + const types = { + ECDSAMetaTransaction: [ + { name: 'msgData', type: 'bytes' }, + { name: 'msgValue', type: 'uint256' }, + { name: 'nonce', type: 'uint256' }, + ], + }; + + const values = { + msgData, + msgValue, + nonce, + }; + + const signature = await signer.signTypedData(domain, types, values); - return permit; + return ethers.Signature.from(signature); }; -export { signERC2612Permit }; +export { signERC2612Permit, signECDSAMetaTransaction }; diff --git a/test/meta/ECDSAMetaTransactionContext.ts b/test/meta/ECDSAMetaTransactionContext.ts new file mode 100644 index 00000000..9b943691 --- /dev/null +++ b/test/meta/ECDSAMetaTransactionContext.ts @@ -0,0 +1,250 @@ +import { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/signers'; +import { signECDSAMetaTransaction } from '@solidstate/library'; +import { + $ECDSAMetaTransactionContext, + $ECDSAMetaTransactionContext__factory, +} from '@solidstate/typechain-types'; +import { TypedContractMethod } from '@solidstate/typechain-types/common'; +import { expect } from 'chai'; +import { BytesLike, ContractMethodArgs } from 'ethers'; +import { ethers } from 'hardhat'; + +const callMetaTransaction = async ( + signer: SignerWithAddress, + fn: TypedContractMethod<[], [string], 'nonpayable' | 'payable' | 'view'>, + data: BytesLike, + args: ContractMethodArgs<[]> = [], +) => { + const tx = await fn.populateTransaction(...args); + tx.data = ethers.concat([tx.data, data]); + + const result = await signer.call(tx); + + return new ethers.Interface([fn.fragment]).decodeFunctionResult( + fn.name, + result, + ); +}; + +// TODO: support msg.value in helper functions + +const sendMetaTransaction = async ( + signer: SignerWithAddress, + fn: TypedContractMethod<[], [string], 'nonpayable' | 'payable' | 'view'>, + data: BytesLike, + args: ContractMethodArgs<[]> = [], +) => { + const tx = await fn.populateTransaction(...args); + tx.data = ethers.concat([tx.data, data]); + + return await signer.sendTransaction(tx); +}; + +describe('ECDSAMetaTransactionContext', () => { + let instance: $ECDSAMetaTransactionContext; + let deployer: SignerWithAddress; + let forwarder: SignerWithAddress; + let signer: SignerWithAddress; + + beforeEach(async () => { + [deployer, forwarder, signer] = await ethers.getSigners(); + instance = await new $ECDSAMetaTransactionContext__factory( + deployer, + ).deploy(); + }); + + // TODO: spec + + // TODO: test multiple calls in same tx to validate that transient storage is used correctly + + describe('__internal', () => { + describe('#_msgSender()', () => { + it('returns signer if signature is valid', async () => { + const data = instance.$_msgSender.fragment.selector; + const nonce = 1n; + const value = 0n; + + const signature = await signECDSAMetaTransaction( + instance, + signer, + data, + value, + nonce, + ); + + const suffix = ethers.concat([ + await signer.getAddress(), + signature.serialized, + ]); + + expect( + await callMetaTransaction(forwarder, instance.$_msgSender, suffix), + ).to.deep.equal([await signer.getAddress()]); + }); + + it('returns message sender if signature is invalid', async () => { + const suffix = ethers.randomBytes( + Number(await instance.$_calldataSuffixLength.staticCall()), + ); + + expect( + await callMetaTransaction(forwarder, instance.$_msgSender, suffix), + ).to.deep.equal([await forwarder.getAddress()]); + }); + + it('returns message sender if message data length is less than suffix length', async () => { + // account for 4-byte selector when calculting suffix length + const suffix = ethers.randomBytes( + Number( + (await instance.$_calldataSuffixLength.staticCall()) - 4n - 1n, + ), + ); + + expect( + await callMetaTransaction(forwarder, instance.$_msgSender, suffix), + ).to.deep.equal([await forwarder.getAddress()]); + }); + + it('returns incorrect signer if message value is incorrect', async () => { + const data = instance.$_msgSender.fragment.selector; + const nonce = 1n; + const value = 1n; + + const signature = await signECDSAMetaTransaction( + instance, + signer, + data, + value, + nonce, + ); + + const suffix = ethers.concat([ + await signer.getAddress(), + signature.serialized, + ]); + + // a valid address is returned, but it is not the correct signer + + expect( + await callMetaTransaction(forwarder, instance.$_msgSender, suffix), + ).not.to.deep.equal([await signer.getAddress()]); + }); + + describe('reverts if', () => { + it('nonce has been used', async () => { + const data = instance.$_msgSender.fragment.selector; + const nonce = 1n; + const value = 0n; + + const signature = await signECDSAMetaTransaction( + instance, + signer, + data, + value, + nonce, + ); + + const suffix = ethers.concat([ + await signer.getAddress(), + signature.serialized, + ]); + + await sendMetaTransaction(forwarder, instance.$_msgSender, suffix); + + await expect( + callMetaTransaction(forwarder, instance.$_msgSender, suffix), + ).to.be.revertedWith('TODO'); + }); + }); + }); + + describe('#_msgData()', () => { + it('returns message data without suffix if signature is valid', async () => { + const nonSuffixedData = instance.$_msgData.fragment.selector; + const nonce = 1n; + const value = 0n; + + const signature = await signECDSAMetaTransaction( + instance, + signer, + nonSuffixedData, + value, + nonce, + ); + + const suffix = ethers.concat([ + await signer.getAddress(), + signature.serialized, + ]); + + expect( + await callMetaTransaction(forwarder, instance.$_msgData, suffix), + ).to.deep.equal([nonSuffixedData]); + }); + + it('returns complete message data if signature is invalid', async () => { + const nonSuffixedData = instance.$_msgData.fragment.selector; + const suffix = ethers.randomBytes( + Number(await instance.$_calldataSuffixLength.staticCall()), + ); + + // message data is returned as received, demonstrating the malleability of msg.data + + expect( + await callMetaTransaction(forwarder, instance.$_msgData, suffix), + ).to.deep.equal([ethers.concat([nonSuffixedData, suffix])]); + }); + + it('returns complete message data if message data length is less than suffix length', async () => { + const nonSuffixedData = instance.$_msgData.fragment.selector; + // account for 4-byte selector when calculting suffix length + const suffix = ethers.randomBytes( + Number( + (await instance.$_calldataSuffixLength.staticCall()) - 4n - 1n, + ), + ); + + // message data is returned as received, demonstrating the malleability of msg.data + + expect( + await callMetaTransaction(forwarder, instance.$_msgData, suffix), + ).to.deep.equal([ethers.concat([nonSuffixedData, suffix])]); + }); + + describe('reverts if', () => { + it('nonce has been used', async () => { + const data = instance.$_msgData.fragment.selector; + const nonce = 1n; + const value = 0n; + + const signature = await signECDSAMetaTransaction( + instance, + signer, + data, + value, + nonce, + ); + + const suffix = ethers.concat([ + await signer.getAddress(), + signature.serialized, + ]); + + await sendMetaTransaction(forwarder, instance.$_msgData, suffix); + + await expect( + callMetaTransaction(forwarder, instance.$_msgData, suffix), + ).to.be.revertedWith('TODO'); + }); + }); + }); + + describe('#_calldataSuffixLength()', () => { + it('returns 85', async () => { + expect(await instance.$_calldataSuffixLength.staticCall()).to.equal( + 85n, + ); + }); + }); + }); +});