sherlock-audit / 2023-02-hats-judging

2 stars 0 forks source link

duc - Signers can have a free signature to execute transaction of safe if address(0) if a valid wearer. #94

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

duc

medium

Signers can have a free signature to execute transaction of safe if address(0) if a valid wearer.

Summary

As sponsor confirmed, address(0) can receive hats (when isEgilible of address(0) is true). The fact is that address(0) can't claim signer because of the conditions in contract OwnerManageer. However, signers can have a free signature (from address(0)) to execute safe's action.

Vulnerability Detail

Function countValidSignatures doesn't check if currentOwner is address(0).

(v, r, s) = signatureSplit(signatures, i);
    if (v == 0) {
        // If v is 0 then it is a contract signature
        // When handling contract signatures the address of the contract is encoded into r
        currentOwner = address(uint160(uint256(r)));
    } else if (v == 1) {
        // If v is 1 then it is an approved hash
        // When handling approved hashes the address of the approver is encoded into r
        currentOwner = address(uint160(uint256(r)));
    } else if (v > 30) {
        // If v > 30 then default va (27,28) has been adjusted for eth_sign flow
        // To support eth_sign and similar we adjust v and hash the messageHash with the Ethereum message prefix before applying ecrecover
        currentOwner =
            ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
    } else {
        // Default is the ecrecover flow with the provided data hash
        // Use ecrecover with the messageHash for EOA signatures
        currentOwner = ecrecover(dataHash, v, r, s);
    }

    if (isValidSigner(currentOwner)) {
        // shouldn't overflow given reasonable sigCount
        unchecked {
            ++validSigCount;
        }
    }

Example if the signature is 0x00..0, then v = 0 and currentOwner is address(0). If address(0) is a wearer of the valid hat, the signers can have a free signature easily to execute transaction.

Impact

Signers can have a free signature

Code Snippet

https://github.com/Hats-Protocol/hats-zodiac/blob/9455cc0957762f5dbbd8e62063d970199109b977/src/HatsSignerGateBase.sol#L561-L564

Tool used

Manual review

Recommendation

Should not increase validSigCount if owner is address(0)

Duplicate of #39