You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Jun 11, 2023. It is now read-only.
sherlock-admin opened this issue
Mar 9, 2023
· 1 comment
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHSGMediumA valid Medium severity issueRewardA payout will be made for this issue
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) publicvirtual {
if (isValidSigner(_signer)) {
revertStillWearsSignerHat(_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.
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHSGMediumA valid Medium severity issueRewardA payout will be made for this issue
Dug
medium
Valid signers can be forcibly removed from the safe
Summary
By calling
removeOwner
orswapOwner
on thesafe
, 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 withisValidSigner
, reverting if the subject is wearing a signer hat.However, a subset of signers can bypass these checks by approving a transaction that calls
removeOwner
orswapOwner
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
The text was updated successfully, but these errors were encountered: