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

`_validateSignature` and `validateUserOp` do not follow official ERC4337 Standard

abhishekthakur

Summary

_validateSignature do not follow official ERC4337 standard

Vulnerability Details

In MondrianWallet smartcontract, It's supposed be fully compatible with ERC4337. But it do not follow it strictly which can cause whole batch to revert. Which is not the intended behaviour by EIP4337.

ffunction _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash)
internal
pure
returns (uint256 validationData)
{
bytes32 hash = MessageHashUtils.toEthSignedMessageHash(userOpHash);
@> ECDSA.recover(hash, userOp.signature);
return SIG_VALIDATION_SUCCESS;
}

Here the function can revert in highlighted line if signature is invalid. As per official ERC4337, It shouldn't revert on invalid signature, rather return SIG_VALIDATION_FAILED.

In validateUserOp there is uint256 return value, which is currently fixed to 0 if signature is valid else it's gonna revert. But as per official ERC4337 the return value for validateUserOp must be packed of authorizer, validUntil and validAfter timestamps.

  • authorizer - 0 for valid signature, 1 to mark signature failure. Otherwise, an address of an authorizer contract. This ERC defines “signature aggregator” as authorizer.

  • validUntil is 6-byte timestamp value, or zero for “infinite”. The UserOp is valid only up to this time.

  • validAfter is 6-byte timestamp. The UserOp is valid only after this time.

Impact

  • Revert on signature mismatch, gonna cause reverting of whole batch. (As most of transactions are processed in batches in AA).

  • Signature will be valid for infinite time can cause, signature replay attacks.

  • Incomptability in Signature Aggregators

Tools Used

Manual Review

Recommendations

Adhere to standard ERC4337 to avoid these bugs. These are some of the recomendations.

function _validateSignature(PackedUserOperation calldata userOp, bytes32 userOpHash)
internal
pure
returns (uint256 validationData)
{
bytes32 hash = MessageHashUtils.toEthSignedMessageHash(userOpHash);
address user = ECDSA.recover(hash, userOp.signature);
if(user != owner()) {
return SIG_VALIDATION_FAILED;
}
// Calculate validUntil and validAfter timestamps
uint48 validUntil = 0; // Set to zero for "indefinite" validity
uint48 validAfter = 0; // No delay by default
return uint256(user) << 208 | uint256(validUntil) << 160 | uint256(validAfter) << 112;
}
}
```
Updates

Lead Judging Commences

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

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

validationData return MUST be a return composed of `authorizer`, `validUntil` and `validAfter` timestamps

Support

FAQs

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