signature is not validated

[hats-bug-reporter[bot]]

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0xe458bcaaf6e4d0173186894301d6a2213144dc069174e3805c03629524149211 Severity: low

Description: Description\

SignatureValidator.sol is base contract supporting ERC1271 differentisValidSignature versions.

_verifySignature() is overridden and has been used in SafeWebAuthnSharedSigner.sol and SafeWebAuthnSignerSingleton.sol.

    function _verifySignature(bytes32 message, bytes calldata signature) internal view virtual returns (bool success);

Then, this _verifySignature() is being called in isValidSignature()

    function isValidSignature(bytes32 message, bytes calldata signature) external view returns (bytes4 magicValue) {
        if (_verifySignature(message, signature)) {
            magicValue = ERC1271.MAGIC_VALUE;
        }
    }

The issue is that, _verifySignature() as internal function does not validate the signature length passed with it. To protetct from malicious signatures, the signature length should be checked.

If we see, EIP-1271 the reference implementation has checked the signature length thereby validating the signature input explicitely.

    require(_signature.length == 65, "SignatureValidator#recoverSigner: invalid signature length");

Reference- Reference Implementation

The means the signature.length == 65 considers, signature inputs, v as bytes1, r and s is bytes32 of signature.

Recommendations\

For signature inputs in functions, Check the signature length.

For example:

    require(signature.length == 65, "invalid signature length");

[nlordell]

I am marking this as invalid as the signature bytes are checked in WebAuthn.sol (which is ultimately called by all paths of isValidSignature and includes checks on upper bounds of signature length encoding:

https://github.com/hats-finance/Safe-0x2909fdefd24a1ced675cb1444918fa766d76bdac/blob/2c03e35cd1f93de704136ab6e54ae42971a69465/modules/passkey/contracts/libraries/WebAuthn.sol#L121

Note that this is an upper bound check by design:

https://github.com/hats-finance/Safe-0x2909fdefd24a1ced675cb1444918fa766d76bdac/blob/2c03e35cd1f93de704136ab6e54ae42971a69465/modules/passkey/contracts/libraries/WebAuthn.sol#L88-L91

The proposed solution is not valid for this project - WebAuthn signatures are not 65 bytes, they include two 32 byte values for the ECDSA signature R and S components, as well as bytes for authenticator and client data that is required for computing the signing message.