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

3 stars 3 forks source link

Missing input validation on the lengths of _firstCohort and _secondCohort to ensure they conform to the documentation's specification #243

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/SecurityCouncilManager.sol#L100 https://github.com/ArbitrumFoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/SecurityCouncilManager.sol#L101

Vulnerability details

Impact

The function initialize checks that the lengths of _firstCohort and _secondCohort are equal but does not validate that their lengths are equal to the documented value of 6. The documentation states that each cohort should have 6 members, which means both _firstCohort and _secondCohort should have a length of 6.

Proof of Concept

If this validation is not in place, there's a risk that cohorts could be initialized with incorrect sizes, leading to potential logic errors or other unforeseen consequences in other parts of the system. This can lead to data inconsistency and unexpected behaviors.

Tools Used

Manual

Recommended Mitigation Steps

Add input validation checks for the length of _firstCohort and _secondCohort:

require(_firstCohort.length == 6, "First cohort length must be 6");
require(_secondCohort.length == 6, "Second cohort length must be 6");

Assessed type

Context

0xSorryNotSorry commented 1 year ago

Could be QA as the end-user or the protocol is not affected of an existing issue but a potential one.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xean marked the issue as grade-c