Signature verification is not implemented for Legacy EIP-1271

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x1019c255aab9a19a1e670c220c98061a64a5c573ca47d330c4a14197c58c2fd6 Severity: low

Description: Description\ SignatureValidator has two functions to validate the signatures. One with actual EIP-2171 MAGIC_VALUE and another with EIP-2171 LEGACY_MAGIC_VALUE.

It can be seen in isValidSignatureForSigner() function of SafeWebAuthnSignerFactory.sol:

    function isValidSignatureForSigner(
        bytes32 message,
        bytes calldata signature,
        uint256 x,
        uint256 y,
        P256.Verifiers verifiers
    ) external view override returns (bytes4 magicValue) {
        address singleton = address(SINGLETON);
        bytes memory data = abi.encodePacked(
@>          abi.encodeWithSignature("isValidSignature(bytes32,bytes)", message, signature),
            x,
            y,
            verifiers
        );

        // solhint-disable-next-line no-inline-assembly
        assembly {
            // staticcall to the singleton contract with return size given as 32 bytes. The
            // singleton contract is known and immutable so it is safe to specify return size.
            if staticcall(gas(), singleton, add(data, 0x20), mload(data), 0, 32) {
                magicValue := mload(0)
            }
        }
    }

There is only functionality for EIP-2171 signature verification with MAGIC_VALUE, However, the legacy version LEGACY_MAGIC_VALUE implemented in SignatureValidator contract is not used anywhere.

    function isValidSignature(bytes memory data, bytes calldata signature) external view returns (bytes4 magicValue) {
        if (_verifySignature(keccak256(data), signature)) {
            magicValue = ERC1271.LEGACY_MAGIC_VALUE;
        }
    }

I believe, since there is inclusion of LEGACY_MAGIC_VALUE in Safe contracts then for sure there would be its use to validate the signature which is in form bytes4(keccak256("isValidSignature(bytes,bytes)").

Recommendation\ Check and review the use of LEGACY_MAGIC_VALUE across inscope safe contracts. As only the EIP-2171 is being used to validate the signature and this legacy version is used un-left.

[nlordell]

Note that isValidSignatureForSigner is a bespoke function for validating signatures given signer parameters is not an EIP-1271 or legacy EIP-1271 method. As such, it does not need to adhere to any particular standard.

In fact, the function is documented to return the same value as createSigner().isValidSignature() which implies that it should return the EIP-1271 magic value.

As such, I believe this submission is invalid.