Summary
The validateUserOp() function is expected to validate user's signature and nonce but it doesn't.
Vulnerability Details
The validateUserOp() function inherits the documentation from the interface IAccount.sol#L7-L33.
* Validate user's signature and nonce
* the entryPoint will make the call to the recipient only if this validation call returns successfully.
* signature failure should be reported by returning SIG_VALIDATION_FAILED (1).
* This allows making a "simulation call" without a valid signature
* Other failures (e.g. nonce mismatch, or invalid signature format) should still revert to signal failure.
*
* @dev Must validate caller is the entryPoint.
* Must validate the signature and nonce
* @param userOp - The operation that is about to be executed.
* @param userOpHash - Hash of the user's request data. can be used as the basis for signature.
* @param missingAccountFunds - Missing funds on the account's deposit in the entrypoint.
* This is the minimum amount to transfer to the sender(entryPoint) to be
* able to make the call. The excess is left as a deposit in the entrypoint
* for future calls. Can be withdrawn anytime using "entryPoint.withdrawTo()".
* In case there is a paymaster in the request (or the current deposit is high
* enough), this value will be zero.
* @return validationData - Packaged ValidationData structure. use `_packValidationData` and
* `_unpackValidationData` to encode and decode.
* <20-byte> sigAuthorizer - 0 for valid signature, 1 to mark signature failure,
* otherwise, an address of an "authorizer" contract.
* <6-byte> validUntil - Last timestamp this operation is valid. 0 for "indefinite"
* <6-byte> validAfter - First timestamp this operation is valid
* If an account doesn't use time-range, it is enough to
* return SIG_VALIDATION_FAILED value (1) for signature failure.
* Note that the validation code cannot use block.timestamp (or block.number) directly.
*/
function validateUserOp(
The code of validateUserOp() method function is:
function validateUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash, uint256 missingAccountFunds)
external
virtual
override
requireFromEntryPoint
returns (uint256 validationData)
{
validationData = _validateSignature(userOp, userOpHash);
_validateNonce(userOp.nonce);
_payPrefund(missingAccountFunds);
}
The validation takes place inside the function _validateSignature() method, let's also take a look into that:
function _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;
}
The function _validateSignature uses ECDSA.recover(hash, userOp.signature) to recover the address, but it DOES NOT use the recovered address to compare against the value of userOp.sender.
Therefore _validateSignature() returns always SIG_VALIDATION_SUCCESS, this is incorrect.
Going forward, the validation of nonce is made inside the function _validateNonce()
which is EMPTY, so there's no validation.
function _validateNonce(uint256 nonce) internal view virtual {}
Impact
The function validateUserOp() method isn't able to catch any mismatching nor invalid signature.
Tools Used
The code has been visual inspected
Recommendations
The _validateSignature function should be changed as follows:
function _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;
+ if (userOp.sender == ECDSA.recover(hash, userOp.signature) return SIG_VALIDATION_SUCCESS;
+ return SIG_VALIDATION_FAILED
}
The function `_validateNonce()' should be written from scratch.
