sherlock-audit / 2023-02-hats-judging

2 stars 0 forks source link

cducrest-brainbot - Increase the number of valid signers past maxSigners #130

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

cducrest-brainbot

high

Increase the number of valid signers past maxSigners

Summary

The number of valid signers can be increased past maxSigners if users are added, their hats are toggled of so they are considered invalid signers, new signers are added reaching the maxSigners limit, and the hats of the previously inactive signers are toggled back on.

Vulnerability Detail

reconcileSignerCount() does not remove inactive owners from the underlying safe, it simply updates the threshold and the signerCount value.

New users can be added with respect to the maxSigners constraint after signers have been made invalid by toggling their hats off and calling reconcileSignerCount().

Once the hats of the inactive users are turned back on, the number of valid signers on the underlying safe will go above maxSigners and checks like if (currentSignerCount >= maxSigs) { revert ... } will revert.

Impact

The number of signers on the underlying safe may go above limits resulting in failing safe transaction / reconcileSignerCount() due to maxSigs checks.

Code Snippet

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

Tool used

Manual Review

Recommendation

Duplicate of #51