HatsSignerGate + MultiHatsSignerGate: more than maxSignatures can be claimed which leads to DOS in reconcileSignerCount
The `HatsSignerGate.claimSigner` and `MultiHatsSignerGate.claimSigner` functions allow users to become signers.\\nIt is important that both functions do not allow that there exist more valid signers than `maxSigners`.\\nThis is because if there are more valid signers than `maxSigners`, any call to `HatsSignerGateBase.reconcileSignerCount` reverts, which means that no transactions can be executed.\\nThe only possibility to resolve this is for a valid signer to give up his signer hat. No signer will voluntarily give up his signer hat. And it is wrong that a signer must give it up. Valid signers that have claimed before `maxSigners` was reached should not be affected by someone trying to become a signer and exceeding `maxSigners`. In other words the situation where one of the signers needs to give up his signer hat should have never occurred in the first place.\\nThink of the following scenario:\\n`maxSignatures=10` and there are 10 valid signers\\nThe signers execute a transaction that calls `Safe.addOwnerWithThreshold` such that there are now 11 owners (still there are 10 valid signers)\\nOne of the 10 signers is no longer a wearer of the hat and `reconcileSignerCount` is called. So there are now 9 valid signers and 11 owners\\nThe signer that was no longer a wearer of the hat in the previous step now wears the hat again. However `reconcileSignerCount` is not called. So there are 11 owners and 10 valid signers. The HSG however still thinks there are 9 valid signers.\\nWhen a new signer now calls `claimSigner`, all checks will pass and he will be swapped for the owner that is not a valid signer:\\n```\\n        // 9 >= 10 is false\\n        if (currentSignerCount >= maxSigs) {\\n            revert MaxSignersReached();\\n        }\\n\\n        // msg.sender is a new signer so he is not yet owner\\n        if (safe.isOwner(msg.sender)) {\\n            revert SignerAlreadyClaimed(msg.sender);\\n        }\\n\\n        // msg.sender is a valid signer, he wears the signer hat\\n        if (!isValidSigner(msg.sender)) {\\n            revert NotSignerHatWearer(msg.sender);\\n        }\\n```\\n\\nSo there are now 11 owners and 11 valid signers. This means when `reconcileSignerCount` is called, the following lines cause a revert:\\n```\\n    function reconcileSignerCount() public {\\n        address[] memory owners = safe.getOwners();\\n        uint256 validSignerCount = _countValidSigners(owners);\\n\\n        // 11 > 10\\n        if (validSignerCount > maxSigners) {\\n            revert MaxSignersReached();\\n        }\\n```\\n
The `HatsSignerGate.claimSigner` and `MultiHatsSignerGate.claimSigner` functions should call `reconcileSignerCount` such that they work with the correct amount of signers and the scenario described in this report cannot occur.\\n```\\ndiff --git a/src/HatsSignerGate.sol b/src/HatsSignerGate.sol\\nindex 7a02faa..949d390 100644\\n--- a/src/HatsSignerGate.sol\\n// Add the line below\\n// Add the line below\\n// Add the line below\\n b/src/HatsSignerGate.sol\\n@@ -34,6 // Add the line below\\n34,8 @@ contract HatsSignerGate is HatsSignerGateBase {\\n     /// @notice Function to become an owner on the safe if you are wearing the signers hat\\n     /// @dev Reverts if `maxSigners` has been reached, the caller is either invalid or has already claimed. Swaps caller with existing invalid owner if relevant.\\n     function claimSigner() public virtual {\\n// Add the line below\\n        reconcileSignerCount();\\n// Add the line below\\n\\n         uint256 maxSigs = maxSigners; // save SLOADs\\n         uint256 currentSignerCount = signerCount;\\n```\\n\\n```\\ndiff --git a/src/MultiHatsSignerGate.sol b/src/MultiHatsSignerGate.sol\\nindex da74536..57041f6 100644\\n--- a/src/MultiHatsSignerGate.sol\\n// Add the line below\\n// Add the line below\\n// Add the line below\\n b/src/MultiHatsSignerGate.sol\\n@@ -39,6 // Add the line below\\n39,8 @@ contract MultiHatsSignerGate is HatsSignerGateBase {\\n     /// @dev Reverts if `maxSigners` has been reached, the caller is either invalid or has already claimed. Swaps caller with existing invalid owner if relevant.\\n     /// @param _hatId The hat id to claim signer rights for\\n     function claimSigner(uint256 _hatId) public {\\n// Add the line below\\n        reconcileSignerCount();\\n// Add the line below\\n        \\n         uint256 maxSigs = maxSigners; // save SLOADs\\n         uint256 currentSignerCount = signerCount;\\n```\\n
As mentioned before, we end up in a situation where one of the valid signers has to give up his signer hat in order for the HSG to become operable again.\\nSo one of the valid signers that has rightfully claimed his spot as a signer may lose his privilege to sign transactions.
```\\n        // 9 >= 10 is false\\n        if (currentSignerCount >= maxSigs) {\\n            revert MaxSignersReached();\\n        }\\n\\n        // msg.sender is a new signer so he is not yet owner\\n        if (safe.isOwner(msg.sender)) {\\n            revert SignerAlreadyClaimed(msg.sender);\\n        }\\n\\n        // msg.sender is a valid signer, he wears the signer hat\\n        if (!isValidSigner(msg.sender)) {\\n            revert NotSignerHatWearer(msg.sender);\\n        }\\n```\\n