Submitted by dirk_y
https://github.com/ArbitrumFoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/SecurityCouncilManager.sol#L183-L190
https://github.com/ArbitrumFoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/SecurityCouncilManager.sol#L161-L173
https://github.com/ArbitrumFoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/SecurityCouncilManager.sol#L176-L180
https://github.com/ArbitrumFoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/SecurityCouncilManager.sol#L143-L159
As stated in the updated constitution text:
Security Council members may only be removed prior to the end of their terms under two conditions:
However, since the Security Council are the only party with the ability to add a member, they can simply re-add the member that was removed by a DAO vote if they disagree with the vote. This entirely defeats the point of giving the DAO the ability to remove a member of the council mid-term.
A member of the Security Council can be removed mid-term by calling removeMember in SecurityCouncilManager.sol. This can only be called by addresses that have the MEMBER_REMOVER_ROLE role; the SecurityCouncilMemberRemovalGovernor.sol contract and the 9 of 12 emergency Security Council:
The actual member removal is performed by _removeMemberFromCohortArray:
As you can see, the member that has been removed isnt tracked/stored anywhere for later querying. So, lets assume that the DAO has successfully voted to remove a member. The removeMember method is called when the vote is successful and the update is schedule in the L2 timelock.
The Security Council now have the ability (as the only party with the MEMBER_ADDER_ROLE role) to add a new member to fill the relevant cohort on the council by calling addMember:
Where _addMemberToCohortArray looks like:
As you can see, this method only checks to ensure the member being added doesnt already exist in either cohort, however there is no validation whether or not this member has been removed before in the current term. As a result, any DAO votes to remove a member can be usurped by the Security Council since they can simply restate the member that was voted to be removed.
Any member removals mid-term should be tracked for the lifecycle of the term (6 months) to ensure that the same member cant be re-added. This array should be wiped when a cohort is replaced with a call to replaceCohort.
Invalid Validation
yahgwai (Arbitrum) acknowledged and commented:
dirk_y (warden) commented:
