diff --git a/manager/4337/README.md b/manager/4337/README.md new file mode 100644 index 0000000..7cbeb05 --- /dev/null +++ b/manager/4337/README.md @@ -0,0 +1,116 @@ +# Safe{Core} Protocol ERC-4337 Support + +## Compatibility + +Safe{Core} Protocol specification is meant to be compatible with [ERC-4337](https://eips.ethereum.org/EIPS/eip-4337). ERC-4337 enforces rules on the read/write operations that should be carefully looked into for the Safe{Core} Protocol implementation to be compatible with ERC-4337. + +As per ERC-4337 specs state the following when validating a `UserOperation`: + +> Storage access is limited as follows: +> +> - self storage (of factory/paymaster, respectively) is allowed, but only if self entity is staked +> - account storage access is allowed (see Storage access by Slots, below), +> - in any case, may not use storage used by another UserOp sender in the same bundle (that is, paymaster and factory are not allowed as senders + +Storage associated with an account is defined as follows: + +> An address `A` is associated with: +> +> - Slots of contract `A` address itself. +> - Slot `A` on any other address. +> - Slots of type `keccak256(A || X) + n` on any other address. (to cover `mapping(address => value)`, which is usually used for balance in ERC-20 tokens). `n` is an offset value up to 128, to allow accessing fields in the format `mapping(address => struct)` + +As such, in order for the Safe{Core} Protocol to be ERC-4337 compatible without requiring a staked _paymaster_, the Manager MUST NOT make use of storage slots not associated with the account whose `UserOperation`s it is validating. Notably, this implies that `UserOperation` validation MUST be implemented without interacting with the Registry. If ERC-4337 `validateUserOp` support is implemented as a module, the Manager MUST NOT verify it is approved in the Registry. Note that this restriction only applies to `UserOperation` validation flow and notably does not apply to the `UserOperation` execution flow. The specification currently suggests that ERC-4337 `validateUserOp` SHOULD be directly supported in the Manager for these compatibility reasons. + +In particular, the storage access limitations during `UserOperation` validation can be problematic when checking a module against the Registry as it is required to retrieve information associated with a module address (and not the account address). There are potential workarounds using contract code as storage [\[1\]](https://github.com/5afe/contract-storage) that would a Registry implementation to still validate a ERC-4337 module during the `UserOperation` validation phase, but they rely on `SELFDESTRUCT` which is a deprecated op-code and hence not recommended. + +Additionally, developers aiming to develop plugins that are ERC-4337 compatible should be aware of storage access restrictions, opcode usage restrictions during the simulation step of ERC-4337 specification. + +More details on this is available [here](https://github.com/safe-global/safe-core-protocol/issues/60#issuecomment-1761296305). + +### Staking + +The account SHOULD implement a staked _factory_ in order to enable `UserOperations` with `initCode`. ERC-4337 requires a staked _factory_ for `UserOperation` validation that reads associated account storage when `initCode` is specified. Since this specification permits Manager implementations that read account associated storage during `UserOperation` validation, the account _factory_ needs to be staked in order for _bundlers_ to accept `UserOperations` with `initCode`. + +## Security Considerations + +One of the main value propositions of the Safe{Core} Protocol is account security by verifying modules in the Registry. However, because of storage access restrictions imposed by the ERC-4337 spec (see Compatibility section above), this is not possible when validating user operations. However, this restriction does not apply to `UserOperation` execution. + +In order to help protect accounts from any eventual vulnerabilities that may be discovered in the ERC-4337 _entrypoint_, The manager MUST validate `UserOperation` execution with the registry for the particular _entrypoint_ being used. Practically speaking this implies that the ERC-4337 `UserOperation` execution: +1. SHOULD be implemented as a fallback handler module with an immutable associated _entrypoint_ contract +2. MUST verify that the _entrypoint_ has been explicitly enabled by the account +3. MUST verify that the _entrypoint_ has not been flagged + +Note that if the `UserOperation` execution is implemented as a module that only supports a single _entrypoint_, then the standard Registry check that the Manager is required to do for modules suffices for points 2 and 3 above. Otherwise the Manager MUST check that the _entrypoint_ has been enabled by the user and not been flagged through some other means. A sequence diagram detailing the required checks based on the recommended implementation is included in the Execution Flow section below. + +By leveraging the Safe{Core} Protocol to verify whether or not _entrypoints_ are approved, the attack surface caused by an _entrypoint_ compromise is greatly reduced. In fact, the attack is limited to the Ether transfer from the `UserOperation` validation. + +### Additional Security With `SELFDESTRUCT` + +While a `validateUserOp` function handler module cannot verify if it is still approved in the registry, it MAY provide a mechanism for `SELFDESTRUCT`-ing the module's `validateUserOp` function implementation to further limit the potential damages in case a particular _entrypoint_ vulnerability is discovered. Note that this is not required, especially since the `SELFDESTRUCT` op-code is officially deprecated and may stop working in the future. Therefore, this is not a reliable way to protect `UserOperation` validation from _entrypoint_ vulnerabilities. + +## Using ERC-4337 + +This section contains sequence diagrams for using ERC-4337 support in the Safe{Core} Protocol. **It assumes that ERC-4337 is implemented as a module**, but with additional special case support for ERC-4337 in the Manager (see Compatibility section for more details). + +### Enabling ERC-4337 + +Enabling ERC-4337 on an account the following registrations for the ERC-4337 module implementation: + +1. Fallback handler for `validateUserOp` and `executeUserOp` functions +2. Plugin (with or without root access, depending on what transactions the account wants to permit over ERC-4337). + +### Validation Flow + +```mermaid +sequenceDiagram + participant E as EntryPoint + participant A as Account + participant M as Manager + participant Mod as ERC-4337 Module + + E->>+A: validateUserOp() + A-->>+M: validateUserOp() + Note over M: check if account has validateUserOp fallback handler enabled + M->>+Mod: handle(validateUserOp) + Note over Mod: ensure EntryPoint is supported + Mod->>A: checkSignatures() + Mod->>M: transferPrefund(account, entrypoint, missingAccountFunds) + M->>A: executeTransactionFromModule(transfer{missingAccountFunds}) + A->>E: transfer{missingAccountFunds} + Mod-->>-M: validationData + M-->>-A: validationData + A-->>-E: validationData +``` + +### Execution Flow + +```mermaid +sequenceDiagram + participant E as Entry Aoint + participant A as Account + participant M as Manager + participant R as Registry + participant Mod as ERC-4337 Module + actor T as Target + + E->>+A: executeUserOp() + A-->>+M: executeUserOp() + Note over M: check if account has executeUserOp fallback handler enabled + M->>+R: check(module) + R-->>-M: flaggedAt + Note over M: ensure fallback handler is not flagged + M->>+Mod: handle(executeUserOp) + Note over Mod: ensure EntryPoint is supported + Mod->>+M: executeSafeTransction(account, userOp) + M->>+R: check(module) + R-->>-M: flaggedAt + Note over M: ensure plugin is not flagged + M->>+A: executeTransactionFromModule(userOp) + A->>T: userOp + A-->>-M: result + M-->>-Mod: result + Mod-->>-M: result + M-->>-A: result + A-->>-E: result +``` diff --git a/manager/README.md b/manager/README.md index 3203fce..c696d28 100644 --- a/manager/README.md +++ b/manager/README.md @@ -105,39 +105,6 @@ TBD It is inevitable that more features will be added to Safe{Core} Protocol (e.g. new modules). As the Manager is the central part of this setup, it is important to consider a path for integrating these new features. Using an upgradeable proxy for the Manager would introduce unacceptable security concerns. Separating too much of the functionality into separate contract to allow reusability (i.e. the list of enabled integration) would increase the gas costs, and so is also not practical. A better pattern is to allow new versions of the Manager to load information from a previous version and thereby facilitate a migration. -## ERC-4337 compatibility +## ERC-4337 -Safe{Core} Protocol specification is meant to be compatible with [ERC-4337](https://eips.ethereum.org/EIPS/eip-4337). ERC-4337 enforces rules on the the read/write operations that should be carefully looked into for the Safe{Core} Protocol implementation to be compatible with ERC-4337. - -As per ERC-4337 specs state the following when simulating a `UserOp` : - -``` -Storage access is limited as follows: -- self storage (of factory/paymaster, respectively) is allowed, but only if self entity is staked -- account storage access is allowed (see Storage access by Slots, below), -- in any case, may not use storage used by another UserOp sender in the same bundle (that is, paymaster and factory are not allowed as senders - -Storage associated with an account is defined as follows: - -An address A is associated with: -- Slots of contract A address itself. -- Slot A on any other address. -- Slots of type keccak256(A || X) + n on any other address. (to cover mapping(address => value), which is usually used for balance in ERC-20 tokens). n is an offset value up to 128, to allow accessing fields in the format mapping(address => struct) -``` - -The current Protocol Manager implementations specifies following operations that make use of storage slots that are not allowed by ERC-4337: - -- Read the registry address from storage slot not associated with the account - - When executing transaction from Plugin - - When hooks are enabled - - When validating signature -- Registry contract reads the storage associated with module address - -Potential solutions: - -- Avoid registry storage reads -- Adjust data structures to avoid reading storage not associated with account address - -Developers aiming to develop plugins that are ERC-4337 compatible should be aware of storage access restrictions, opcode usage restrictions during the simulation step of ERC-4337 specification. - -More details on this is available here: https://github.com/safe-global/safe-core-protocol/issues/60#issuecomment-1761296305 \ No newline at end of file +The Safe{Core} Protocol is designed to extend accounts with ERC-4337 support. Managers MUST implement the required ERC-4337 functionality outlined in the [ERC-4337](./4337/README.md) page. diff --git a/modules/README.md b/modules/README.md index dac4590..020fbdb 100644 --- a/modules/README.md +++ b/modules/README.md @@ -34,7 +34,7 @@ interface ISafeProtocolPlugin { } ``` -### Plugin permissions +### Plugin Permissions The table below elaborates the permission types that a plugin can have. For each transaction, the `Manager` will check if the plugin has the required permission. Each permission should be granted by the account explicitly. There is no hierarchy or precedence order for the permissions. @@ -62,7 +62,7 @@ Inspired from [EIP-6617](https://eips.ethereum.org/EIPS/eip-6617), must `require ## Hooks -### Hooks for transaction execution +### Hooks for Transaction Execution Hooks add additional logic at certain points of the transaction lifecycle. Hooks enable various forms of security protections such as allow- and deny-lists, MEV-protections, risk-assessments, and more. The Safe{Core} Protocol currently recognizes the following types of hooks: - `preCheck` / `preCheckRootAccess` verify custom conditions using the state before a transaction is executed @@ -87,7 +87,7 @@ interface ISafeProtocolHooks { | Multisignature Flow (for accounts that use [Guard interface](https://github.com/safe-global/safe-contracts/blob/8ffae95faa815acf86ec8b50021ebe9f96abde10/contracts/base/GuardManager.sol#L10)) | Encoded data created from parameter values received from `checkTransaction(...)` i.e. `abi.encode(to, value, data, gas, baseGas, gasPrice, gasToken, refundReceiver, signatures, msgSender)` | | Plugin Flow | Encoded address of the Plugin i.e. `abi.encode(pluginAddress)` | -#### Parameter `executionType` value +#### Parameter `executionType` Value | Execution Type | Value | |---------------------|-------| @@ -96,9 +96,9 @@ interface ISafeProtocolHooks { TODO: provide more details on execution metadata -## Function handler - -Non-static version (invoked via `call`) +## Function Handler +d +Non-static version (invoked via `call`): ```solidity interface ISafeProtocolFunctionHandler { @@ -106,7 +106,7 @@ interface ISafeProtocolFunctionHandler { } ``` - function metadataProvider() external view returns (uint256 type, bytes memory location); +Static version (invoked via `staticcall`): ```solidity interface ISafeProtocolStaticFunctionHandler { @@ -114,14 +114,16 @@ interface ISafeProtocolStaticFunctionHandler { } ``` +Function handlers, once installed are executed without checking the registry. + Kudos to @mfw78 -## Signature validators +## Signature Validators -There are continuous efforts to expand the types of signatures supported by the EVM beyond the currently predominant secp256k1 elliptic curve. For example, a signature scheme gaining popularity is based on the secp256r1 elliptic curve (see EIP-7212). Signature Validators allow accounts to support new standards and enable use-cases such as Passkeys-enabled smart accounts, BLS/Schnorr or quantum-secure signatures. +Signature validators enable modules to extend an account's [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) `isValidSignature` implementation allowing for custom signature validation rules. Inspired from [EIP-712](https://eips.ethereum.org/EIPS/eip-712) which specifies standard for typed structured data hashing and signing, a signature validator is expected to validate a signed message for a specific domain. To do so, a `SignatureValidatorManager` contract acts as storage for maintaining enabled validators (approved by the registry) per domain per account or use a default signature validation scheme. -Apart from validating EIP-712 typed signed message an account might also require to support other arbitrary signed messages for interaction with projects following other message hashing and signing formats. Other signing standards can be supported using default signature validation scheme which depends on the account implementation. +Apart from validating EIP-712 typed signed message an account might also be required to support other arbitrary signed messages for interaction with projects following other message hashing and signing formats. Other signing standards can be supported using default signature validation scheme which depends on the account implementation. ### Enabling Signature Validator @@ -129,13 +131,13 @@ For enabling a domain specific EIP-712 signature validator, first a signature va ```mermaid sequenceDiagram - participant Account - participant SignatureValidatorManager - participant RegistryContract + participant Account + participant SignatureValidatorManager + participant RegistryContract - Account->>SignatureValidatorManager: Set SignatureValidatorContract for a specific domain. - SignatureValidatorManager->>RegistryContract: Check if SignatureValidatorContract is listed and not flagged - RegistryContract-->>SignatureValidatorManager: Return result + Account->>SignatureValidatorManager: Set SignatureValidatorContract for a specific domain. + SignatureValidatorManager->>RegistryContract: Check if SignatureValidatorContract is listed and not flagged + RegistryContract-->>SignatureValidatorManager: Return result SignatureValidatorManager->>SignatureValidatorManager: Check if SignatureValidatorContract implements ISafeProtocolSignatureValidator interface SignatureValidatorManager-->>Account: Ok ``` @@ -146,21 +148,21 @@ The possible cases for a transaction to revert are: - Signature validator contract is flagged in registry - Transaction ran out of gas -### Validating account signature +### Validating Account Signature After a enabling a signature validator and setting `SignatureValidatorManager` as function handler in `SafeProtocolManager`, external entities can request validating account signatures. The diagram below illustrates the sequence of calls that are made when an account signature is to be validated. When a signature for an account is to be validated by an external entity, here referred as `ExternalContract`, the `ExternalContract` contract calls the `isValidSignature(bytes32,bytes)` function supported by the account. Account further calls `SignatureValidatorManager`. `SignatureValidatorManager` decides to use default validation scheme or domain specific validator based on the contents of the received data. If domain specific signature validator is to be used and the signature validator contract is set for the account, listed and not flagged, the `SignatureValidatorManager` calls the `SignatureValidatorContract` to check if the signature is valid. Additionally, hooks for signature validators help to provide a way to add additional checks before and after the calling a signature validation function. ```mermaid sequenceDiagram - participant ExternalContract - participant Account + participant ExternalContract + participant Account participant SafeProtocolManager - participant SignatureValidatorManager - participant RegistryContract - participant SignatureValidatorContract + participant SignatureValidatorManager + participant RegistryContract + participant SignatureValidatorContract participant SignatureValidatorHooks - ExternalContract->>Account: Check if signature is valid for the account (Call isValidSignature(bytes32,bytes)) + ExternalContract->>Account: Check if signature is valid for the account (Call isValidSignature(bytes32,bytes)) Account->>SafeProtocolManager: SafeProtocolManager enabled as fallback handler for account SafeProtocolManager->>SignatureValidatorManager: Call handle(...) function SignatureValidatorManager->>SignatureValidatorManager: Decode data and extract 4 bytes signature selector @@ -182,7 +184,7 @@ sequenceDiagram Account-->>SignatureValidatorManager: Ok end SignatureValidatorManager->>SignatureValidatorHooks: Execute postValidationHook(...) if enabled - SignatureValidatorManager-->>ExternalContract: Return signature validation result + SignatureValidatorManager-->>ExternalContract: Return signature validation result Account-->>ExternalContract: Return signature validation result ``` @@ -255,7 +257,7 @@ In the default signature validation flow, the wallet is expected to generate a h In the above code, the account's domain separator is included in the message whose hash is signed. By doing so, the account only approves relevant signatures and not any other arbitrary signed messages. The domain separator is expected to include the account address and chainId to avoid cross-chain replay attacks. The actual value domain separator depends on the account implementation. -### Signature validator interface +### Signature Validator Interface A signature validator must implement the following interface. @@ -281,7 +283,7 @@ interface ISafeProtocolSignatureValidator { } ``` -### Hooks for signature validator +### Hooks for Signature Validator Signature validation is a critical part of smart contract accounts. A flawed implementation of validation logic can lead to approving a transaction with an invalid signature. Signature validator hooks can be used to add safety net to the signature validation process and hold the power to block a signature validator from approving unauthorized actions. Hooks can be enabled for all validators before and after execution of the validation function. This can be done by adding contract implementing `ISignatureValidatorHook` interface to the `SignatureValidatorManager` by the account. The hooks are not specific to a domain and are executed for all signature validations. The hooks function(s) should revert on failed checks. @@ -304,13 +306,12 @@ interface ISignatureValidatorHooks { } ``` -### Signature validator manager +### Signature Validator Manager A signature validator manager must implement the following interface. ```solidity interface ISafeProtocolSignatureValidatorManager { - /** * @param domainSeparator bytes32 containing the domain for which Signature Validator contract should be used * @param signatureValidatorContract Address of the Signature Validator Contract implementing ISafeProtocolSignatureValidator interface @@ -324,7 +325,7 @@ interface ISafeProtocolSignatureValidatorManager { } ``` -### Security considerations +### Security Considerations A malicious actor can change the routing of the signature validation flow by adding/removing the 4 bytes of the signature selector. Under such cases, the signature validation should fail and revert the transaction. diff --git a/registry/README.md b/registry/README.md index af7e199..659b739 100644 --- a/registry/README.md +++ b/registry/README.md @@ -20,6 +20,7 @@ interface SafeProtocolRegistry { ``` ## Hyperstructures and Authorities + For a detailed discussion on Hyperstructures and Authorities and their role in the protocol, refer to this [article](https://mirror.xyz/konradkopp.eth/7Q3TrMFgx2VbZRKa7UEaisIMjimpMABiqGYo00T9egA). ## Automation