Biconomy: Nexus

Summary

Nexus.sol signature validation process would be broken in some cases

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/modules/validators/K1Validator.sol#L99-L109

https://github.com/Cyfrin/2024-07-biconomy/blob/9590f25cd63f7ad2c54feb618036984774f3879d/contracts/types/Constants.sol#L16-L19

Evidently, EIP 1271 is being implemented as the SignatureChecker which allows contracts to sign messages and works great in tandem with EIP 4337 (account abstraction).

Now for ERC-4337, the account is not deployed until the first UserOp is mined, previous to this, the account exists "counterfactually" — it has an address even though it's not really deployed. Usually, this is great since we can use the counterfactual address to receive assets without deploying the account first.

Not being able to sign messages from counterfactual contracts/accounts has always been a limitation of ERC-1271 since one can't call the _isValidSignature() function on them.

So this practically means that Biconomy will fail to validate signatures for users of notorious wallets/projects, including Safe, Sequence, and Argent supporting ERC1271, but also ERC4337 wallets, even though a clear intention has been made to support signatures by EIP1271 compliant wallets, as confirmed by using the Eip-1271 method of validating signatures.

Crux of the issue is the fact that protocol is taking responsibility to check the validity of signatures, but partially failing to trigger signature validation signatures for a group of wallets from a provider since (the validation will succeed if the ERC4337 wallet is deployed) and given that the protocol decided to support contract-based wallets (that support ERC4337) and implement ERC1271, one could assume that they "inherit" the possibility from the wallet providers to support ERC4337 too.

Impact

This vulnerability easily impacts Biconomy's ability to validate signatures for counterfactual ERC-4337 accounts, limiting the usability for users of certain wallets that rely on AA, leading to the limitation of functionalities for Nexus.sol, since all operations that need the signatures attached to the typehashes wouldn't work for some set of users, i.e the availability of these functions is impacted.

Since this functionality ends up being used in Nexus.sol, here & here.

Tools Used

Manual review

Recommended Mitigation Steps

EIP-6492 solves this issue. The author of the EIP already implemented ERC-6492 in a universal library which verifies 6492, 712, and 1271 sigs with this pull request.

ERC6492 assumes that the signing contract will normally be a contract wallet, but it could be any contract that implements ERC-1271 and is deployed counterfactually.

• If the contract is deployed, produce a normal ERC-1271 signature

• If the contract is not deployed yet, wrap the signature as follows: concat(abi.encode((create2Factory, factoryCalldata, originalERC1271Signature), (address, bytes, bytes)), magicBytes)

Biconomy could adopt the reference-implementation as stated in the EIP and delegate the responsibility of supporting counterfactual signatures to the wallets themselves, and this works because, as defined in the EIP, the wallet should return the magic value in both cases.