From c3aacc664c477c8fd81ee6e51b132aa49a9398b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Tue, 10 Dec 2024 09:22:50 -0600 Subject: [PATCH] Test linearization fail --- contracts/account/draft-AccountBase.sol | 143 ++++++++++++ contracts/account/draft-AccountECDSA.sol | 97 ++++++++ .../cryptography/draft-ERC7739Signer.sol | 129 +++++++++++ .../utils/cryptography/draft-ERC7739Utils.sol | 218 ++++++++++++++++++ lib/forge-std | 2 +- 5 files changed, 588 insertions(+), 1 deletion(-) create mode 100644 contracts/account/draft-AccountBase.sol create mode 100644 contracts/account/draft-AccountECDSA.sol create mode 100644 contracts/utils/cryptography/draft-ERC7739Signer.sol create mode 100644 contracts/utils/cryptography/draft-ERC7739Utils.sol diff --git a/contracts/account/draft-AccountBase.sol b/contracts/account/draft-AccountBase.sol new file mode 100644 index 000000000..f30ba5f69 --- /dev/null +++ b/contracts/account/draft-AccountBase.sol @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {PackedUserOperation, IAccount, IEntryPoint, IAccountExecute} from "../interfaces/draft-IERC4337.sol"; +import {Address} from "../utils/Address.sol"; + +/** + * @dev A simple ERC4337 account implementation. + * + * This base implementation only includes the minimal logic to process user operations. + * Developers must implement the {_validateUserOp} function to define the account's validation logic. + */ +abstract contract AccountBase is IAccount, IAccountExecute { + /** + * @dev Unauthorized call to the account. + */ + error AccountUnauthorized(address sender); + + /** + * @dev Revert if the caller is not the entry point or the account itself. + */ + modifier onlyEntryPointOrSelf() { + _checkEntryPointOrSelf(); + _; + } + + /** + * @dev Revert if the caller is not the entry point. + */ + modifier onlyEntryPoint() { + _checkEntryPoint(); + _; + } + + /** + * @dev Canonical entry point for the account that forwards and validates user operations. + */ + function entryPoint() public view virtual returns (IEntryPoint) { + return IEntryPoint(0x0000000071727De22E5E9d8BAf0edAc6f37da032); + } + + /** + * @dev Return the account nonce for the canonical sequence. + */ + function getNonce() public view virtual returns (uint256) { + return getNonce(0); + } + + /** + * @dev Return the account nonce for a given sequence (key). + */ + function getNonce(uint192 key) public view virtual returns (uint256) { + return entryPoint().getNonce(address(this), key); + } + + /** + * @dev Returns the digest the offchain signer signed instead of the opaque `userOpHash`. + * + * Given the `userOpHash` calculation is defined by ERC-4337, offchain signers + * may need to sign again this hash by rehashing it with other schemes. + * + * Returns the `userOpHash` by default. + */ + function _userOpSignedHash( + PackedUserOperation calldata /* userOp */, + bytes32 userOpHash + ) internal view virtual returns (bytes32) { + return userOpHash; + } + + /** + * @inheritdoc IAccount + */ + function validateUserOp( + PackedUserOperation calldata userOp, + bytes32 userOpHash, + uint256 missingAccountFunds + ) public virtual onlyEntryPoint returns (uint256) { + uint256 validationData = _validateUserOp(userOp, _userOpSignedHash(userOp, userOpHash)); + _payPrefund(missingAccountFunds); + return validationData; + } + + /** + * @inheritdoc IAccountExecute + */ + function executeUserOp( + PackedUserOperation calldata userOp, + bytes32 /*userOpHash*/ + ) public virtual onlyEntryPointOrSelf { + (address target, uint256 value, bytes memory data) = abi.decode(userOp.callData[4:], (address, uint256, bytes)); + Address.functionCallWithValue(target, data, value); + } + + /** + * @dev Validation logic for {validateUserOp}. The `userOpSignedHash` is the digest from {_userOpSignedHash}. + * + * IMPORTANT: Implementing a mechanism to validate user operations is a security-sensitive operation + * as it may allow an attacker to bypass the account's security measures. Check out {AccountECDSA}, + * {AccountP256}, or {AccountRSA} for digital signature validation implementations. + */ + function _validateUserOp( + PackedUserOperation calldata userOp, + bytes32 userOpSignedHash + ) internal view virtual returns (uint256 validationData); + + /** + * @dev Sends the missing funds for executing the user operation to the {entrypoint}. + * The `missingAccountFunds` must be defined by the entrypoint when calling {validateUserOp}. + */ + function _payPrefund(uint256 missingAccountFunds) internal virtual { + if (missingAccountFunds > 0) { + (bool success, ) = payable(msg.sender).call{value: missingAccountFunds}(""); + success; // Silence warning. The entrypoint should validate the result. + } + } + + /** + * @dev Ensures the caller is the {entrypoint}. + */ + function _checkEntryPoint() internal view virtual { + address sender = msg.sender; + if (sender != address(entryPoint())) { + revert AccountUnauthorized(sender); + } + } + + /** + * @dev Ensures the caller is the {entrypoint} or the account itself. + */ + function _checkEntryPointOrSelf() internal view virtual { + address sender = msg.sender; + if (sender != address(this) && sender != address(entryPoint())) { + revert AccountUnauthorized(sender); + } + } + + /** + * @dev Receive Ether. + */ + receive() external payable virtual {} +} diff --git a/contracts/account/draft-AccountECDSA.sol b/contracts/account/draft-AccountECDSA.sol new file mode 100644 index 000000000..dcaef5a18 --- /dev/null +++ b/contracts/account/draft-AccountECDSA.sol @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {PackedUserOperation} from "../interfaces/draft-IERC4337.sol"; +import {ERC4337Utils} from "../account/utils/draft-ERC4337Utils.sol"; +import {ERC721Holder} from "../token/ERC721/utils/ERC721Holder.sol"; +// import {ERC1155HolderLean, IERC1155Receiver} from "../token/ERC1155/utils/ERC1155HolderLean.sol"; +import {ERC1155Holder, IERC1155Receiver} from "../token/ERC1155/utils/ERC1155Holder.sol"; +import {ERC165} from "../utils/introspection/ERC165.sol"; +import {IERC165} from "../utils/introspection/IERC165.sol"; +import {ECDSA} from "../utils/cryptography/ECDSA.sol"; +import {MessageHashUtils} from "../utils/cryptography/MessageHashUtils.sol"; +import {AccountBase} from "./draft-AccountBase.sol"; +import {ERC7739Signer, EIP712} from "../utils/cryptography/draft-ERC7739Signer.sol"; +import {Initializable} from "../proxy/utils/Initializable.sol"; + +/** + * @dev Account implementation using {ECDSA} signatures and {ERC7739Signer} for replay protection. + * + * An {_initializeSigner} function is provided to set the account's signer address. Doing so it's + * easier for a factory, whose likely to use initializable clones of this contract. + * + * IMPORTANT: Avoiding to call {_initializeSigner} either during construction (if used standalone) + * or during initialization (if used as a clone) may leave the account unusable. + */ +abstract contract AccountECDSA is ERC165, ERC721Holder, ERC1155Holder, ERC7739Signer, AccountBase { + using MessageHashUtils for bytes32; + + /** + * @dev The {signer} is already initialized. + */ + error AccountECDSAUninitializedSigner(address signer); + + address private _signer; + + /** + * @dev Initializes the account with the address of the native signer. This function is called only once. + */ + function _initializeSigner(address signerAddr) internal { + if (_signer != address(0)) revert AccountECDSAUninitializedSigner(signerAddr); + _signer = signerAddr; + } + + /** + * @dev Return the account's signer address. + */ + function signer() public view virtual returns (address) { + return _signer; + } + + /** + * @dev Returns the ERC-191 signed `userOpHash` hashed with keccak256 using `personal_sign`. + */ + function _userOpSignedHash( + PackedUserOperation calldata /* userOp */, + bytes32 userOpHash + ) internal view virtual override returns (bytes32) { + return userOpHash.toEthSignedMessageHash(); + } + + /** + * @dev Internal version of {validateUserOp} that relies on {_validateSignature}. + * + * The `userOpSignedHash` is the digest from {_userOpSignedHash}. + * + * NOTE: To override the signature functionality, try overriding {_validateSignature} instead. + */ + function _validateUserOp( + PackedUserOperation calldata userOp, + bytes32 userOpSignedHash + ) internal view virtual override returns (uint256) { + return + _isValidSignature(userOpSignedHash, userOp.signature) + ? ERC4337Utils.SIG_VALIDATION_SUCCESS + : ERC4337Utils.SIG_VALIDATION_FAILED; + } + + /** + * @dev Validates the signature using the account's signer. + * + * This function provides a nested EIP-712 hash. Developers must override only this + * function to ensure no raw message signing is possible. + */ + function _validateSignature( + bytes32 nestedEIP712Hash, + bytes calldata signature + ) internal view virtual override returns (bool) { + (address recovered, ECDSA.RecoverError err, ) = ECDSA.tryRecover(nestedEIP712Hash, signature); + return signer() == recovered && err == ECDSA.RecoverError.NoError; + } + + /// @inheritdoc ERC165 + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, ERC1155Holder) returns (bool) { + return interfaceId == type(IERC1155Receiver).interfaceId || super.supportsInterface(interfaceId); + } +} diff --git a/contracts/utils/cryptography/draft-ERC7739Signer.sol b/contracts/utils/cryptography/draft-ERC7739Signer.sol new file mode 100644 index 000000000..7fe073d92 --- /dev/null +++ b/contracts/utils/cryptography/draft-ERC7739Signer.sol @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IERC1271} from "../../interfaces/IERC1271.sol"; +import {EIP712} from "../../utils/cryptography/EIP712.sol"; +import {MessageHashUtils} from "../../utils/cryptography/MessageHashUtils.sol"; +import {ShortStrings} from "../../utils/ShortStrings.sol"; +import {ERC7739Utils} from "./draft-ERC7739Utils.sol"; + +/** + * @dev Validates signatures wrapping the message hash in a nested EIP712 type. See {ERC7739Utils}. + * + * Linking the signature to the EIP-712 domain separator is a security measure to prevent signature replay across different + * EIP-712 domains (e.g. a single offchain owner of multiple contracts). + * + * This contract requires implementing the {_validateSignature} function, which passes the wrapped message hash, + * which may be either an typed data or a personal sign nested type. + * + * NOTE: {EIP712} uses {ShortStrings} to optimize gas costs for short strings (up to 31 characters). + * Consider that strings longer than that will use storage, which may limit the ability of the signer to + * be used within the ERC-4337 validation phase (due to ERC-7562 storage access rules). + */ +abstract contract ERC7739Signer is EIP712, IERC1271 { + using ERC7739Utils for *; + using MessageHashUtils for bytes32; + + /** + * @dev Attempts validating the signature in a nested EIP-712 type. + * + * A nested EIP-712 type might be presented in 2 different ways: + * + * - As a nested EIP-712 typed data + * - As a _personal_ signature (an EIP-712 mimic of the `eth_personalSign` for a smart contract) + */ + function isValidSignature(bytes32 hash, bytes calldata signature) public view virtual returns (bytes4 result) { + // For the hash `0x7739773977397739773977397739773977397739773977397739773977397739` and an empty signature, + // we return the magic value too as it's assumed impossible to find a preimage for it that can be used maliciously. + // Useful for simulation purposes and to validate whether the contract supports ERC-7739. + return + _isValidSignature(hash, signature) + ? IERC1271.isValidSignature.selector + : (hash == 0x7739773977397739773977397739773977397739773977397739773977397739 && signature.length == 0) + ? bytes4(0x77390001) + : bytes4(0xffffffff); + } + + /** + * @dev Internal version of {isValidSignature} that returns a boolean. + */ + function _isValidSignature(bytes32 hash, bytes calldata signature) internal view virtual returns (bool) { + return + _isValidNestedTypedDataSignature(hash, signature) || _isValidNestedPersonalSignSignature(hash, signature); + } + + /** + * @dev Nested personal signature verification. + * + * NOTE: Instead of overriding this function, try with {_validateSignature}. It encapsulates + * nested EIP-712 hashes. + */ + function _isValidNestedPersonalSignSignature( + bytes32 hash, + bytes calldata signature + ) internal view virtual returns (bool) { + return _validateSignature(_domainSeparatorV4().toTypedDataHash(hash.personalSignStructHash()), signature); + } + + /** + * @dev Nested EIP-712 typed data verification. + * + * NOTE: Instead of overriding this function, try with {_validateSignature}. It encapsulates + * nested EIP-712 hashes. + */ + function _isValidNestedTypedDataSignature( + bytes32 hash, + bytes calldata encodedSignature + ) internal view virtual returns (bool) { + // decode signature + ( + bytes calldata signature, + bytes32 appSeparator, + bytes32 contentsHash, + string calldata contentsDescr + ) = encodedSignature.decodeTypedDataSig(); + + ( + , + string memory name, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + + ) = eip712Domain(); + + // Check that contentHash and separator are correct + // Rebuild nested hash + return + hash == appSeparator.toTypedDataHash(contentsHash) && + bytes(contentsDescr).length != 0 && + _validateSignature( + appSeparator.toTypedDataHash( + ERC7739Utils.typedDataSignStructHash( + contentsDescr, + contentsHash, + abi.encode(keccak256(bytes(name)), keccak256(bytes(version)), chainId, verifyingContract, salt) + ) + ), + signature + ); + } + + /** + * @dev Signature validation algorithm. + * + * To ensure there's no way to inherit from this ERC-7739 signer and still be able to sign raw messages, + * this function provides a nested EIP-712 hash. Developers must implement only this function to ensure + * no raw message signing is possible. + * + * WARNING: Implementing a signature validation algorithm is a security-sensitive operation as it involves + * cryptographic verification. It is important to review and test thoroughly before deployment. Consider + * using one of the signature verification libraries ({ECDSA}, {P256} or {RSA}). + */ + function _validateSignature( + bytes32 nestedEIP712Hash, + bytes calldata signature + ) internal view virtual returns (bool); +} diff --git a/contracts/utils/cryptography/draft-ERC7739Utils.sol b/contracts/utils/cryptography/draft-ERC7739Utils.sol new file mode 100644 index 000000000..320ac4bd4 --- /dev/null +++ b/contracts/utils/cryptography/draft-ERC7739Utils.sol @@ -0,0 +1,218 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +/** + * @dev Utilities to process https://ercs.ethereum.org/ERCS/erc-7739[ERC-7739] typed data signatures + * that are specific to an EIP-712 domain. + * + * This library provides methods to wrap, unwrap and operate over typed data signatures with a defensive + * rehashing mechanism that includes the application's {EIP712-_domainSeparatorV4} and preserves + * readability of the signed content using an EIP-712 nested approach. + * + * A smart contract domain can validate a signature for a typed data structure in two ways: + * + * - As an application validating a typed data signature. See {toNestedTypedDataHash}. + * - As a smart contract validating a raw message signature. See {toNestedPersonalSignHash}. + * + * NOTE: A provider for a smart contract wallet would need to return this signature as the + * result of a call to `personal_sign` or `eth_signTypedData`, and this may be unsupported by + * API clients that expect a return value of 129 bytes, or specifically the `r,s,v` parameters + * of an {ECDSA} signature, as is for example specified for {EIP712}. + */ +library ERC7739Utils { + /** + * @dev An EIP-712 type to represent "personal" signatures + * (i.e. mimic of `personal_sign` for smart contracts). + */ + bytes32 private constant PERSONAL_SIGN_TYPEHASH = keccak256("PersonalSign(bytes prefixed)"); + + /** + * @dev Error when the contents type is invalid. See {tryValidateContentsType}. + */ + error InvalidContentsType(); + + /** + * @dev Nest a signature for a given EIP-712 type into a nested signature for the domain of the app. + * + * Counterpart of {decodeTypedDataSig} to extract the original signature and the nested components. + */ + function encodeTypedDataSig( + bytes memory signature, + bytes32 appSeparator, + bytes32 contentsHash, + string memory contentsDescr + ) internal pure returns (bytes memory) { + return + abi.encodePacked(signature, appSeparator, contentsHash, contentsDescr, uint16(bytes(contentsDescr).length)); + } + + /** + * @dev Parses a nested signature into its components. + * + * Constructed as follows: + * + * `signature ‖ DOMAIN_SEPARATOR ‖ contentsHash ‖ contentsDescr ‖ uint16(contentsDescr.length)` + * + * - `signature` is the original signature for the nested struct hash that includes the "contents" hash + * - `DOMAIN_SEPARATOR` is the EIP-712 {EIP712-_domainSeparatorV4} of the smart contract verifying the signature + * - `contentsHash` is the hash of the underlying data structure or message + * - `contentsDescr` is a descriptor of the "contents" part of the the EIP-712 type of the nested signature + */ + function decodeTypedDataSig( + bytes calldata encodedSignature + ) + internal + pure + returns (bytes calldata signature, bytes32 appSeparator, bytes32 contentsHash, string calldata contentsDescr) + { + unchecked { + uint256 sigLength = encodedSignature.length; + + if (sigLength < 4) return (_emptyCalldataBytes(), 0, 0, _emptyCalldataString()); + + uint256 contentsDescrEnd = sigLength - 2; // Last 2 bytes + uint256 contentsDescrLength = uint16(bytes2(encodedSignature[contentsDescrEnd:])); + + if (contentsDescrLength + 64 > contentsDescrEnd) + return (_emptyCalldataBytes(), 0, 0, _emptyCalldataString()); + + uint256 contentsHashEnd = contentsDescrEnd - contentsDescrLength; + uint256 separatorEnd = contentsHashEnd - 32; + uint256 signatureEnd = separatorEnd - 32; + + signature = encodedSignature[:signatureEnd]; + appSeparator = bytes32(encodedSignature[signatureEnd:separatorEnd]); + contentsHash = bytes32(encodedSignature[separatorEnd:contentsHashEnd]); + contentsDescr = string(encodedSignature[contentsHashEnd:contentsDescrEnd]); + } + } + + /** + * @dev Nests an `ERC-191` digest into a `PersonalSign` EIP-712 struct, and return the corresponding struct hash. + * This struct hash must be combined with a domain separator, using {MessageHashUtils-toTypedDataHash} before + * being verified/recovered. + * + * This is used to simulates the `personal_sign` RPC method in the context of smart contracts. + */ + function personalSignStructHash(bytes32 contents) internal pure returns (bytes32) { + return keccak256(abi.encode(PERSONAL_SIGN_TYPEHASH, contents)); + } + + /** + * @dev Nests an `EIP-712` hash (`contents`) into a `TypedDataSign` EIP-712 struct, and return the corresponding + * struct hash. This struct hash must be combined with a domain separator, using {MessageHashUtils-toTypedDataHash} + * before being verified/recovered. + */ + function typedDataSignStructHash( + string calldata contentsTypeName, + string calldata contentsType, + bytes32 contentsHash, + bytes memory domainBytes + ) internal pure returns (bytes32 result) { + return + bytes(contentsTypeName).length == 0 + ? bytes32(0) + : keccak256( + abi.encodePacked(typedDataSignTypehash(contentsTypeName, contentsType), contentsHash, domainBytes) + ); + } + + /** + * @dev Variant of {typedDataSignStructHash-string-string-bytes32-string-bytes} that takes a content descriptor + * and decodes the `contentsTypeName` and `contentsType` out of it. + */ + function typedDataSignStructHash( + string calldata contentsDescr, + bytes32 contentsHash, + bytes memory domainBytes + ) internal pure returns (bytes32 result) { + (string calldata contentsTypeName, string calldata contentsType) = decodeContentsDescr(contentsDescr); + + return typedDataSignStructHash(contentsTypeName, contentsType, contentsHash, domainBytes); + } + + /** + * @dev Compute the EIP-712 typehash of the `TypedDataSign` structure for a given type (and typename). + */ + function typedDataSignTypehash( + string calldata contentsTypeName, + string calldata contentsType + ) internal pure returns (bytes32) { + return + keccak256( + abi.encodePacked( + "TypedDataSign(", + contentsTypeName, + " contents,string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)", + contentsType + ) + ); + } + + /** + * @dev Parse the type name out of the ERC-7739 contents type description. Supports both the implicit and explicit + * modes. + * + * Following ERC-7739 specifications, a `contentsTypeName` is considered invalid if it's empty or it contains + * any of the following bytes , )\x00 + * + * If the `contentsType` is invalid, this returns an empty string. Otherwise, the return string has non-zero + * length. + */ + function decodeContentsDescr( + string calldata contentsDescr + ) internal pure returns (string calldata contentsTypeName, string calldata contentsType) { + bytes calldata buffer = bytes(contentsDescr); + if (buffer.length == 0) { + // pass through (fail) + } else if (buffer[buffer.length - 1] == bytes1(")")) { + // Implicit mode: read contentsTypeName for the beginning, and keep the complete descr + for (uint256 i = 0; i < buffer.length; ++i) { + bytes1 current = buffer[i]; + if (current == bytes1("(")) { + // if name is empty - passthrough (fail) + if (i == 0) break; + // we found the end of the contentsTypeName + return (string(buffer[:i]), contentsDescr); + } else if (_isForbiddenChar(current)) { + // we found an invalid character (forbidden) - passthrough (fail) + break; + } + } + } else { + // Explicit mode: read contentsTypeName for the end, and remove it from the descr + for (uint256 i = buffer.length; i > 0; --i) { + bytes1 current = buffer[i - 1]; + if (current == bytes1(")")) { + // we found the end of the contentsTypeName + return (string(buffer[i:]), string(buffer[:i])); + } else if (_isForbiddenChar(current)) { + // we found an invalid character (forbidden) - passthrough (fail) + break; + } + } + } + return (_emptyCalldataString(), _emptyCalldataString()); + } + + // slither-disable-next-line write-after-write + function _emptyCalldataBytes() private pure returns (bytes calldata result) { + assembly ("memory-safe") { + result.offset := 0 + result.length := 0 + } + } + + // slither-disable-next-line write-after-write + function _emptyCalldataString() private pure returns (string calldata result) { + assembly ("memory-safe") { + result.offset := 0 + result.length := 0 + } + } + + function _isForbiddenChar(bytes1 char) private pure returns (bool) { + return char == 0x00 || char == bytes1(" ") || char == bytes1(",") || char == bytes1("(") || char == bytes1(")"); + } +} diff --git a/lib/forge-std b/lib/forge-std index 1eea5bae1..8f24d6b04 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 1eea5bae12ae557d589f9f0f0edae2faa47cb262 +Subproject commit 8f24d6b04c92975e0795b5868aa0d783251cdeaa