The signerCount value is incorrect

Summary

The signerCount value is incorrect

Vulnerability Detail

function _removeSigner(address _signer) internal {
        bytes memory removeOwnerData;
        address[] memory owners = safe.getOwners();
        uint256 currentSignerCount = signerCount; // save an SLOAD
        uint256 newSignerCount;

        if (currentSignerCount < 2 && owners.length == 1) {
            // signerCount could be 0 after reconcileSignerCount
            // make address(this) the only owner
            removeOwnerData = abi.encodeWithSignature(
                "swapOwner(address,address,address)",
                SENTINEL_OWNERS, // prevOwner
                _signer, // oldOwner
                address(this) // newOwner
            );

            // newSignerCount is already 0
        } else {
            uint256 currentThreshold = safe.getThreshold();
            uint256 newThreshold = currentThreshold;
            uint256 validSignerCount = _countValidSigners(owners);

            if (validSignerCount == currentSignerCount) {
                newSignerCount = currentSignerCount;
            } else {
                newSignerCount = currentSignerCount - 1;
            }

            // ensure that txs can't execute if fewer signers than target threshold
            if (newSignerCount <= targetThreshold) {
                newThreshold = newSignerCount;
            }

            removeOwnerData = abi.encodeWithSignature(
                "removeOwner(address,address,uint256)", _findPrevOwner(owners, _signer), _signer, newThreshold
            );
        }

        // update signerCount
        signerCount = newSignerCount;

The removeSigner() is used to removes an invalid signer from the safe. When update the signerCount in this function , the signerCount should be the validSignerCount. The newSignerCount value may not be correct, as some people may not be wearing the hat anymore. The same issue in _grantSigner() and _swapSigner() functions.

Impact

Code Snippet

https://github.com/Hats-Protocol/hats-zodiac/blob/9455cc0957762f5dbbd8e62063d970199109b977/src/HatsSignerGateBase.sol#L404 https://github.com/Hats-Protocol/hats-zodiac/blob/9455cc0957762f5dbbd8e62063d970199109b977/src/HatsSignerGateBase.sol#L285 https://github.com/Hats-Protocol/hats-zodiac/blob/9455cc0957762f5dbbd8e62063d970199109b977/src/HatsSignerGateBase.sol#L343

Tool used

Manual Review

Recommendation

signerCount_= _countValidSigners(owners);

Duplicate of #79

sherlock-audit / 2023-02-hats-judging

Bauer - The signerCount value is incorrect #82