Arbitrum Security Council Election System - 0xTheC0der's results

Lines of code

https://github.com/ArbitrumFoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol#L73-L94

Vulnerability details

Impact

The SecurityCouncilNomineeElectionGovernorTiming.electionToTimestamp(uint256 electionIndex) method basically adds 6 months * electionIndex to the firstNominationStartDate and returns the resulting timetamp.
The issue is that firstNominationStartDate.day is just passed trough without changes, see L89.
As a consquence, depending on the firstNominationStartDate, unsupported/invalid dates. which exceed the "days in month" (examples: 2030-09-31, 2030-02-29). are generated before the timestamp creation which leads to undefined behaviour in DateTimeLib.dateTimeToTimestamp(...), see also DateTimeLib.isSupportedDateTime(...).

The SecurityCouncilNomineeElectionGovernor.createElection() method is directly affected by the above issue since the creation of a new election might fail or be available too early due to relying on a timestamp whose creation was subject to undefined behaviour.

Keep in mind, that although there might be no problem, i.e. valid timestamps, with most invalid dates using the current library version, it is still undefined behaviour and one has to pay extra attention in case of using another version of the DateTimeLib.

Proof of Concept

The following PoC implicitly modifies the existing testCreateElection case to demonstrate the unsupported/invalid date issue. Just apply the diff below and run the tests with forge test -vv --match-contract SecurityCouncilNomineeElectionGovernorTest.

diff --git a/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol b/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol
index c8fd056..8b5ff35 100644
--- a/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol
+++ b/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol
@@ -83,6 +83,16 @@ abstract contract SecurityCouncilNomineeElectionGovernorTiming is
         // add one to make month 1 indexed
         month += 1;
 
+        // PoC: add check as suggested by DateTimeLib
+        require(DateTimeLib.isSupportedDateTime({
+            year: year,
+            month: month,
+            day: firstNominationStartDate.day,
+            hour: firstNominationStartDate.hour,
+            minute: 0,
+            second: 0
+        }), "PoC: Unsupported election DateTime");
+
         return DateTimeLib.dateTimeToTimestamp({
             year: year,
             month: month,
diff --git a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol
index 9f60ba5..5dc99e0 100644
--- a/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol
+++ b/test/security-council-mgmt/governors/SecurityCouncilNomineeElectionGovernor.t.sol
@@ -17,7 +17,8 @@ contract SecurityCouncilNomineeElectionGovernorTest is Test {
 
     SecurityCouncilNomineeElectionGovernor.InitParams initParams =
     SecurityCouncilNomineeElectionGovernor.InitParams({
-        firstNominationStartDate: Date({year: 2030, month: 1, day: 1, hour: 0}),
+        firstNominationStartDate: Date({year: 2030, month: 3, day: 31, hour: 0}),
+        // other: firstNominationStartDate: Date({year: 2029, month: 8, day: 29, hour: 0}),
         nomineeVettingDuration: 1 days,
         nomineeVetter: address(0x11),
         securityCouncilManager: ISecurityCouncilManager(address(0x22)),

Tools Used

VS Code, Foundry

Recommended Mitigation Steps

Use the DateTimeLib.addMonths(...) method to safely add months to a given DateTime or timestamp.

Assessed type

Math