code-423n4 / 2023-08-arbitrum-findings

3 stars 3 forks source link

SecurityCouncilMemberSyncAction : `perform` function can be continually DOSed which will prevent the valid update the members of the gnosis safe #250

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/ArbitrumFoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/SecurityCouncilMemberSyncAction.sol#L31

Vulnerability details

Impact

The _securityCouncil update will be prevented by continuously calling the perform function. Since the function rely on the nonce value, this function can be continuously called and nonce value is updated. This would prevent the valid security council update since the nonce is lesser than the input value.

Proof of Concept

As we can see the perform function is public and anyone can call this.

In order to execute the council update, the calculated nonce value should be greater than previous nonce value.

But, if the _securityCouncil is known, anyone can call with empty _updatedMembers array and update the nonce value.

So, valid update will be prevented due to this.

Tools Used

Manual review.

Recommended Mitigation Steps

Following methods are suggeted.

  1. Allow that this function would be called by the authorized caller.
  2. Ensure that the _updatedMembers is not empty.

Assessed type

Access Control

0xSorryNotSorry commented 1 year ago

As stated at the NATSPEC of the contract;

@notice Action contract for updating security council members. Used by the security council management system.
   Expected to be delegate called into by an Upgrade Executor

Since the contract doesn't have a non-zero state and the Gnosis call is restricted,

Invalid assumption.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Invalid