Lines of code

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

Vulnerability details

Bug Description

For nominee elections, election dates are determined using the the electionToTimestamp() function in the SecurityCouncilNomineeElectionGovernorTiming module.

When SecurityCouncilNomineeElectionGovernor is initialized after deployment, the first election date is stored through __SecurityCouncilNomineeElectionGovernorTiming_init(). Afterwards, electionToTimestamp() will provide the next election timestamp based on the number of elections passed:

SecurityCouncilNomineeElectionGovernorTiming.sol#L75-L94

    function electionToTimestamp(uint256 electionIndex) public view returns (uint256) {
        // subtract one to make month 0 indexed
        uint256 month = firstNominationStartDate.month - 1;

        month += 6 * electionIndex;
        uint256 year = firstNominationStartDate.year + month / 12;
        month = month % 12;

        // add one to make month 1 indexed
        month += 1;

        return DateTimeLib.dateTimeToTimestamp({
            year: year,
            month: month,
            day: firstNominationStartDate.day,
            hour: firstNominationStartDate.hour,
            minute: 0,
            second: 0
        });
    }

As seen from above, electionToTimestamp() works by adding 6 months for every passed election, and then converting the date to a timestamp through Solady's dateTimeToTimestamp().

However, this approach does not account for months that have less days than others.

For example, if the first election was scheduled on 31 August, the next election would be 31 February according to the formula above. However, as February doesn't have 31 days, the day parameter is outside the range supported by dateTimeToTimestamp(), resulting in undefined behavior:

DateTimeLib.sol#L131-L133

    /// Note: Inputs outside the supported ranges result in undefined behavior.
    /// Use {isSupportedDateTime} to check if the inputs are supported.
    function dateTimeToTimestamp(

Therefore, dateTimeToTimestamp() will return an incorrect timestamp.

Impact

If the the first election starts on the 29th to 31st day of the month, dateTimeToTimestamp() could potentially return an incorrect timestamp for subsequent elections.

Proof of Concept

Assume that the first election is brought forward from 15 September 2023 to 31 August 2023. Every alternate election will now be held in February, which creates two problems:

1. The election date for one cohort will not be fixed

If the current year is a leap year, the election that was supposed to be held on February will be one day earlier than a non-leap year. For example:

Since 2024 is a leap year, the second election will be on 2 March 2024.
Since 2025 is not a leap year, the fourth election will be on 3 March 2025.

This becomes a problem as the Arbitrum Constitution states a specific date for the two elections in a year, which is not possible if the scenario above occurs.

2. One term is a few days shorter for a cohort

As mentioned above, if the start date was 31 August 2023, the fourth election will be on 3 March 2025. However, if the first election was on 3 September 2023, the fourth election would still be on 3 March 2025.

This means that the election starts three days earlier for the scenario above, making the term for one cohort a few days shorter than intended.

Coded Proof

The following test demonstrates how leap years affects electionToTimestamp():

// SPDX-License-Identifier: Apache-2.0
pragma solidity 0.8.16;

import "forge-std/Test.sol";
import "src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol";

contract ElectionDates is Test, SecurityCouncilNomineeElectionGovernorTiming {
    function setUp() public initializer {
        // Set first election date to 31 August 2023, 12:00
        __SecurityCouncilNomineeElectionGovernorTiming_init(
            Date({
                year: 2023,
                month: 8,
                day: 31,
                hour: 12
            }),
            0
        );
    }

    function test_electionToTimestampIncorrect() public {
        // First election is on 31 August 2023
        assertEq(electionToTimestamp(0), 1693483200);

        // Second election is on 2 March 2024
        assertEq(electionToTimestamp(1), 1709380800);

        // Fourth election is on 3 March 2025
        assertEq(electionToTimestamp(3), 1741003200);
    }

    // Required override functions
    function COUNTING_MODE() public pure override returns (string memory) {}
    function votingDelay() public view override returns (uint256) {}
    function votingPeriod() public view override returns (uint256) {}
    function quorum(uint256) public view override returns (uint256) {}
    function hasVoted(uint256, address) public view override returns (bool) {}
    function _quorumReached(uint256) internal view override returns (bool) {}
    function _voteSucceeded(uint256) internal view override returns (bool) {}
    function _getVotes(address, uint256, bytes memory) internal view override returns (uint256) {}
    function _countVote(uint256, address, uint8, uint256, bytes memory) internal override {}
}

Recommended Mitigation

Ensure that _firstNominationStartDate.day is never above 28:

SecurityCouncilNomineeElectionGovernorTiming.sol#L41-L48

-       if (!isSupportedDateTime) {
+       if (!isSupportedDateTime || _firstNominationStartDate.day > 28) {
            revert InvalidStartDate(
                _firstNominationStartDate.year,
                _firstNominationStartDate.month,
                _firstNominationStartDate.day,
                _firstNominationStartDate.hour
            );
        }

Additionally, consider storing startTimestamp instead of firstNominationStartDate. With the first election's timestamp, subsequent election timestamps can be calculated using DateTimeLib.addMonths() instead:

    function electionToTimestamp(uint256 electionIndex) public view returns (uint256) { 
        return DateTimeLib.addMonths(startTimestamp, electionIndex * 6);
    }

Using addMonths() ensures that election dates are always fixed, even in the scenario mentioned above.

Assessed type

Other

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

`electionToTimestamp()` might return incorrect timestamps depending on the day of the first election #254