return value of 0 from ecrecover not checked

[hats-bug-reporter[bot]]

Github username: @ShaheenRehman Twitter username: 0x_Shaheen Submission hash (on-chain): 0x9b563a3d33b17e69b4d8eb6bb3010e3fc71c607f3f0dcd13ecfbb80c1d73f985 Severity: medium

Description: Description\ The solidity function ecrecover is used, however the error result of 0 is not checked for. See documentation: https://docs.soliditylang.org/en/v0.8.9/units-and-global-variables.html?highlight=ecrecover#mathematical-and-cryptographic-functions "recover the address associated with the public key from elliptic curve signature or return zero on error. "

               .....
                } else {
                    // "eth_sign_flow" signatures are specified as v > 30 and are handled differently
                    // if not handle like EOA signature
                    (uint8 v1, bytes32 hashData) = v > 30
                        ? (
                            v - 4,
                            keccak256(
                                abi.encodePacked(
                                    "\x19Ethereum Signed Message:\n32", dataHash
                                )
                                )
                        )
                        : (v, dataHash);
                    ///@audit zero address check?
                    signer = ecrecover(hashData, v1, r, s);
                }
                .....

Attack Scenario\ ecrecover zero errors will not gonna be caught

Attachments

1. Proof of Concept (PoC) File https://docs.soliditylang.org/en/v0.8.9/units-and-global-variables.html?highlight=ecrecover#mathematical-and-cryptographic-functions

2. Revised Code File (Optional)

                   signer = ecrecover(hashData, v1, r, s);
                   require(signer != address(0), "Zero ERR")

Issue Instance:

https://github.com/hats-finance/Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be/blob/dfd821e2fd7825c66c079c19be9460238f6e045a/src/Helpers.sol#L168

[0xRizwan]

Should be low severity, contracts should use openzeppelin's ECDSA.sol instead of ecrecover directly.

[alfredolopez80]

Not @0xRizwan is Non-issue because the method processAndSortSignatures validate the address and order the signatures, if any signature is zero address, in this line: https://github.com/hats-finance/Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be/blob/1ac35880b5d45154267788e2db548eaaae0beaa0/src/Helpers.sol#L186 we verify the signer with comparing with all owner of the safe, if any signer is Zero Address the signature is not concatenated.