Description

The PasskeyRegistryModule contract is called from SmartAccount.sol through the validateUserOp function,

function validateUserOp(
    UserOperation calldata userOp,
    bytes32 userOpHash,
    uint256 missingAccountFunds
) external virtual override returns (uint256 validationData) {
    if (msg.sender != address(entryPoint()))
        revert CallerIsNotAnEntryPoint(msg.sender);

    (, address validationModule) = abi.decode(
        userOp.signature,
        (bytes, address)
    );
    if (address(modules[validationModule]) != address(0)) {
        validationData = IAuthorizationModule(validationModule)
            .validateUserOp(userOp, userOpHash);
    } else {
        revert WrongValidationModule(validationModule);
    }
    _validateNonce(userOp.nonce);
    _payPrefund(missingAccountFunds);
}

where userOp.signature is decoded to figure out the address for the module that should do the actual validation. The validateUserOp() function takes in the raw, unprocessed userOp struct (of type UserOperation).

Inside PasskeyRegistryModule.sol, the validateUserOp(userOp, userOpHash) function is just a wrapper for _validateSignature(userOp, userOpHash), which is a wrapper for _verifySignature(userOpHash, userOp.signature). Do note that the userOp.signature element was passed to the last function. This is the exact same value that was decoded in SmartAccount->validateUserOp(), and it contains both the signature data and the validation module address.

The final function, _verifySignature(), starts like this,

function _verifySignature(
    bytes32 userOpDataHash,
    bytes memory moduleSignature
) internal view returns (bool) {
    (
        bytes32 keyHash,
        uint256 sigx,
        uint256 sigy,
        bytes memory authenticatorData,
        string memory clientDataJSONPre,
        string memory clientDataJSONPost
    ) = abi.decode(
            moduleSignature,
            (bytes32, uint256, uint256, bytes, string, string)
        );
    ...
}

where it tries to decode the signature (including the address) as (bytes32, uint256, uint256, bytes, string, string). This will revert because the validation module address is still a part of the decoded blob.

In the SessionKeyManagerModule.sol contract, the validateUserOp() function is implemented correctly

function validateUserOp(
    UserOperation calldata userOp,
    bytes32 userOpHash
) external view virtual returns (uint256) {
    SessionStorage storage sessionKeyStorage = _getSessionData(msg.sender);
    (bytes memory moduleSignature, ) = abi.decode(
        userOp.signature,
        (bytes, address)
    );
    // Here it does `abi.decode(moduleSignature, ...)`
    ...
}

where the address is stripped off before decoding the remainder.

Impact

The module will always revert and is not usable. If it is the only available validation module, no user operations can happen.

Recommendations

Strip off the address like the SessionKeyManagerModule does, and write test cases for the module. Simple test cases can find mistakes such as these earlier. In general, it is good practice to build a rigorous test suite to ensure the system operates securely and as intended.

Remediation

This issue has been acknowledged by Biconomy Labs, and a fix was implemented in commit 5c5a6bfe↗.