* H-01 Invalid aggregate signature
- BLSAccount (and any other account using signature-aggregator) must verify the public key in the initCode (as last 4 words, just as the BLSAggregator checks)
- validateSignatures must be called before (not after) all validateUserOp
This prevents creating an account that succeeds in simulation, but fails in when executed on-chain (by using different signature during creation)
notes
- the test for simulateHandleOp now can be complete (previously, it was
very difficult/impossible to test for side-effects...
- added a "numberMarker" just before the call, so that tracing it can
easily distinguish execution from post-execution call
* Add an execution method to Safe that reverts if a call fails, so that EntryPoint can catch the revert and emit UserOperationRevertReason
* Add a test for execAndRevert
Reading creation code from the type "GnosisSafeProxy" opens up the risk that the creation code from the imported library is different than the one actually deployed on-chain. Fortunately, we can read the creation code from the proxy factory itself: 767ef36bba/contracts/proxies/GnosisSafeProxyFactory.sol (L33)
The way it's worded right now, it sounds like the account needs to verify the `userOpHash`, when in fact it doesn't have to, since the entryPoint is trusted. Just re-organizing the text to make it clearer.
* AA-101: validate gasLimit to leave more than callGasLimit to target call.
Fix vulnerability reported by Richard Meisner in PR #162
The added tests should show the scenario where a user operation with a high callGasLimit is submitted. In this case it is important that the gasLimit is correctly set, else it is possible to use the 1/64th rule of EIP-150 to make the user operation fail and the account pays for it.
If this is possible it could be used as an attack vector. The attacker would submit a bundle with the high gas usage tx with a too low gas value. Even when the user estimated everything correctly the transaction would fail because not enough gas is available. The costs for the execution would still be deducted from the account. Therefore the submitter could perform a denial of service attack for which the account that is being attacked would pay.
The first tests below shows that high gas transaction can be executed and refunded. The second test checks that the transaction is reverted in case the gas limit is set too low, to avoid the attack described above.
seems that allowing keccak(X||OWN) opens an attack surface (shared
storage between accounts)
rule 3 alone is enough to access balances and allowances of 2 entties
(e.g. paymaster and account) - which is OK
1. A new API for an account (or paymaster) to report signature check validation: instead of revert, they should return `SIG_VALIDATION_FAILED` (1) to mark signature validation. checking for `tx.origin` as zero is no longer in use.
2. ValidateUserOp returns the SIG_VALIDATION_FAILED value (like any other deadline) for a bundler to check.
3. `handleOps` reverts on such signature failure.
4. a new `simulateHandleOp()` helper method, which executes a full UserOp (validation+execution).
like `validateUserOp`, it reverts so it never commits changes on-chain, and doesn't check for signatures, and only used for off-chain simulation. This way, a userOp can be "estimated", even if it depends on the deployed account (e.g. it is a method call on a newly deployed account, or depends on the account's state such as its balance)
* fix factories
BLSFactory use same model as SimpleAccount, using immutable wallet and
only user-specific params in initializer
add factory for TestAggregatedAccount sapmle contract
Create2Factory - use arachnid's de-facto standard deployer, instead of of
the nonstandard EIP2470 (specifically, arachnid's deployer revert on errors)
* gnosis account factory
now Gnosis-Safe based account uses only standard gnosis contracts. The new GnosisSafeAcccountFactory only wraps the standard GnosisSafeProxyFactory to create the proxy (and initialize it with our modules)
check verificationGas on all operations (create, validate, paymaster)
report descriptive error on out-of-gas in validateUserOp, validatePaymasterUserOp
AA-74: sample account cleanup part 1 - using ERC1967Proxy instead of instance
* Remove the 'updateEntryPoint' API as it is not needed - the
'setImplementation' flow should take care of this.
* Make EntryPoint an immutable member of the SimpleAccount implementation
* Merge 'exec' and 'execFromEntryPoint' into single 'execute' method
* Remove '_requireFromAdmin' and 'transfer' functions
* use factory in getAccountInitCode/getAccountAddress, not implementation and ERC1967Proxy
Co-authored-by: Dror Tirosh <dror@opengsn.org>
* reputation (and stake) mechanism to apply to all entities (factory, paymaster, aggregator and account)
* event change: actualGasUsed instead of actualGasPrice
* all entities (sender, factory, paymaster, aggregator) return stake info from simulateValidation
* use "Factory" instead of "deployer"
* emit SignatureAggregatorChanged, AccountDeployed to help bundler with reputation calculation.
Co-authored-by: Yoav Weiss <yoavw@users.noreply.github.com>
wallet is expected to silently ignore signature failure in case the request came from:address(0)
this way, we can implement eth_callUserOp without explicit use acceptance.