Skip to content
This repository has been archived by the owner on Jun 11, 2023. It is now read-only.

Dug - Valid signers can be forcibly removed from the safe #70

Closed
sherlock-admin opened this issue Mar 9, 2023 · 1 comment
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label HSG Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 9, 2023

Dug

medium

Valid signers can be forcibly removed from the safe

Summary

By calling removeOwner or swapOwner on the safe, signers can remove other valid signers from the safe.

Vulnerability Detail

The HSG is set up to prevent valid signers from being removed from the the safe. This is evident in the removeSigner function where a check is made with isValidSigner, reverting if the subject is wearing a signer hat.

    function removeSigner(address _signer) public virtual {
        if (isValidSigner(_signer)) {
            revert StillWearsSignerHat(_signer);
        }

        _removeSigner(_signer);
    }

However, a subset of signers can bypass these checks by approving a transaction that calls removeOwner or swapOwner on the safe.

Impact

This can have political consequences where, if a group of signers meets the threshold, they can remove opposing voices from voting power, potentially replacing them with more aligned signers.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add additional checks in checkAfterExecution that ensures signers we're not removed or swapped.

Duplicate of #118

@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Mar 12, 2023
@github-actions github-actions bot reopened this Mar 12, 2023
@github-actions github-actions bot added Medium A valid Medium severity issue and removed Excluded Excluded by the judge without consulting the protocol or the senior labels Mar 12, 2023
@spengrah spengrah added HSG Disagree With (non-)Duplication The sponsor disputed the duplication state of this issue labels Mar 23, 2023
@spengrah
Copy link

This is a duplicate of #118

@sherlock-admin sherlock-admin added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Reward A payout will be made for this issue labels Mar 29, 2023
@hrishibhat hrishibhat removed the Disagree With (non-)Duplication The sponsor disputed the duplication state of this issue label Apr 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label HSG Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

3 participants