First Flight #15: Mondrian Wallet
First Flight #15
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

The `MondrianWallet::_validateSignature` is not EIP-4337 compliant, which can lead to EntryPoint incompatibilities

0xb0k0

Summary

According to MondrianWallet's documentation (taken from the wallet docs),

We do not use the Paymaster or Aggregator in our Mondrian Wallet.

The protocol does not currently support aggregation, and as per the EIP-4337 documentation,

If the account does not support signature aggregation, it MUST validate the signature is a valid signature of the userOpHash, and SHOULD return SIG_VALIDATION_FAILED (and not revert) on signature mismatch. Any other error should revert.

Currently, the wallet uses the base logic of the ECDSA.recover() function to validate the signature, which reverts in case of a signature mismatch. This can lead to EntryPoint incompatibilities, as any EIP-4337 compatible EntryPoint would expect a return value in case of a signature mismatch.

Impact

The current implementation of the MondrianWallet::_validateSignature function can lead to EntryPoint incompatibilities with EIP-4337 compatible contracts.

Proof of Concept

If we check the implementation of the actual AA signature validation in the SimpleAccount sample, we can see that if there is a signature mismatch, the function returns SIG_VALIDATION_FAILED instead of reverting. This is the expected behavior for EIP-4337 compatible contracts.

function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash)
internal override virtual returns (uint256 validationData) {
bytes32 hash = MessageHashUtils.toEthSignedMessageHash(userOpHash);
@> if (owner != ECDSA.recover(hash, userOp.signature)) // signature mismatch
@> return SIG_VALIDATION_FAILED; // proper return value
return SIG_VALIDATION_SUCCESS;
}

Tools Used

Manual review

Recommended mitigation

Change the MondrianWallet::_validateSignature function to return SIG_VALIDATION_FAILED in case of a signature mismatch.

function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash)
internal
- pure
+ view
returns (uint256 validationData)
{
bytes32 hash = MessageHashUtils.toEthSignedMessageHash(userOpHash);
- ECDSA.recover(hash, userOp.signature);
+ if (owner() != ECDSA.recover(hash, userOp.signature))
+ return SIG_VALIDATION_FAILED;
return SIG_VALIDATION_SUCCESS;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

ECDSA.recover should check against sender

`_validateSignature` SHOULD return SIG_VALIDATION_FAILED (and not revert) on signature mismatch.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.