Alchemy - Multisig Plugin

The `checkNSignatures()` function returns a signature validation failure if the `digest.tryRecover()` call to verify an EOA's signature fails with an error. As a result, the `RecoverError.InvalidSignatureS` (when the `s` value is in the upper range) will be treated as a signature validation failure. This behavior differs from the Modular Account's `SessionKeyPlugin`, which reverts the transaction if a `RecoverError.InvalidSignatureS` error occurs. See [SessionKeyPlugin.sol#L186-L189](https://github.com/alchemyplatform/modular-account/blob/9c2c03197ef4240e32f4d882c4164971e658d639/src/plugins/session/SessionKeyPlugin.sol#L186-L189).

`MultisigModularAccountFactory.getAddress()` reverts in case of `owners.length` exceeds `_MAX_OWNERS_ON_CREATION`. However, `createAccount()` does not perform a similar check, potentially leading to block gas limit-related issues if the number of owners is too high.

The composite signature doesn't specify a maximum length that valid signatures must have. This opens up a potential vector for griefing by appending bytes to the end of the composite signature. Since the `signature` field in the `UserOp` is inherently malleable, a malicious bundler or middle man could insert additional bytes, maxing out the specified `verificationGasLimit `.

According to the plugin manifest, the multisig validation scheme is only expected to be used in the context of a `UserOp`. Therefore, `MultisigPlugin` does not define the runtime validation function for owner-only functions, assuming that any function call in runtime to them will revert due to the lack of validation function. However, another installed plugin can add a runtime validation function to the owner-only functions, allowing them to be executed in runtime. As a result, one may bypass the multisig verification and execute the owner-only functions as long as the runtime verification allows. It may be the intended choice if the account owners want to configure two different verification schemes in `UserOp` and runtime contexts, but it may increase the complexity of managing privileged roles.

When validating a `UserOp`, the multisig plugin's `userOpValidationFunction()` function is called with the corresponding parameters. The plugin compares the gas limits specified in the `UserOp` with the upper limits encoded in the signature. If any gas limits differ, it recalculates the `UserOp` hash with the upper limits by calling `ENTRYPOINT.getUserOpHash()`. However, the function call `getUserOpHash()` to `ENTRYPOINT` is not allowed, as specified in [ERC-7562](https://eips.ethereum.org/EIPS/eip-7562) (a draft EIP for ERC-4337 validation rules). According to OP-051 to OP-054, only calls to the `depositTo()` or fallback function are allowed, and any other function calls are forbidden. Our experiments with the [Infinitism's bundler](https://github.com/eth-infinitism/bundler) showed that the bundler rejects a `UserOp` that calls `ENTRYPOINT.getUserOpHash()` during the validation phase with the following error: ``` Error: processing response error [...] reason: 'processing response error', code: 'SERVER_ERROR', body: '{"jsonrpc":"2.0","id":43,"error":{"message":"illegal call into EntryPoint during validation getUserOpHash","code":-32502}}', ```

The `updateOwnership()` function allows the account owners to update the multisig threshold, specified as the `newThreshold` parameter. If the parameter's value is zero, it indicates the threshold should not be updated. If not, the parameter will be type-casted from `uint256` to `uint128` and stored in the contract. As a result, the final stored threshold value may become zero if the last 128 bits of a positive `newThreshold` are zero, bypassing the multisig-verification scheme after the change since no signature is required anymore.