[Lines of code](https://github.com/arbitrumfoundation/governance/blob/c18de53820c505fc459f766c1b224810eaeaabc5/src/security-council-mgmt/governors/modules/SecurityCouncilNomineeElectionGovernorTiming.sol#L28) # Vulnerability details ## Summary The SecurityCouncilNomineeElectionGovernorTiming contract has an invalid startDate check in the __SecurityCouncilNomineeElectionGovernorTiming_init function. The check ensures that the startDate is in the future, but it does not check that the startDate is greater than the current block timestamp. This means that it is possible for the startDate to be set to a time in the past, which would allow the contract to be initialized with an invalid startDate. ## Description The __SecurityCouncilNomineeElectionGovernorTiming_init function is responsible for initializing the SecurityCouncilNomineeElectionGovernorTiming contract. The function takes two arguments: the first argument is the firstNominationStartDate, which is the start date of the first election, and the second argument is the nomineeVettingDuration, which is the duration of the nominee vetting period. The function first checks to make sure that the firstNominationStartDate is a valid date and time. If it is not, the function reverts with an error. The function then checks to make sure that the firstNominationStartDate is in the future. If it is not, the function reverts with an error. However, the function does not check to make sure that the firstNominationStartDate is greater than the current block timestamp. This means that it is possible for the firstNominationStartDate to be set to a time in the past. If this happens, the contract will initialize successfully, even though the firstNominationStartDate is invalid. ## Impact This bug could be exploited by an attacker to initialize the contract with an invalid firstNominationStartDate. This could allow the attacker to manipulate the election schedule or to prevent elections from happening altogether. For example, the attacker could set the firstNominationStartDate to a time in the past. This would prevent any elections from happening, as the first election would never be triggered. Or, the attacker could set the firstNominationStartDate to a time in the future, but before the current block timestamp. This would allow the attacker to control when the first election is triggered. The attacker could use this to their advantage, for example, by triggering the election at a time when they have a large number of tokens and can easily win the election. ## Proof of Concept pragma solidity 0.8.16; import "../../interfaces/ISecurityCouncilManager.sol"; import "@openzeppelin/contracts-upgradeable/governance/GovernorUpgradeable.sol"; import "solady/utils/DateTimeLib.sol"; import "../../Common.sol"; /// [@title]() SecurityCouncilNomineeElectionGovernorTiming /// [@notice]() Timing module for the SecurityCouncilNomineeElectionGovernor abstract contract SecurityCouncilNomineeElectionGovernorTiming is Initializable, GovernorUpgradeable { /// [@notice]() First election start date Date public firstNominationStartDate; /// [@notice]() Duration of the nominee vetting period (expressed in blocks) /// [@dev]() This is the amount of time after voting ends that the nomineeVetter can exclude noncompliant nominees uint256 public nomineeVettingDuration; error InvalidStartDate(uint256 year, uint256 month, uint256 day, uint256 hour); error StartDateTooEarly(uint256 startTime, uint256 currentTime); /// [@notice]() Initialize the timing module /// [@dev]() Checks to make sure the start date is in the future and is valid function __SecurityCouncilNomineeElectionGovernorTiming_init( Date memory _firstNominationStartDate, uint256 _nomineeVettingDuration ) internal onlyInitializing { bool isSupportedDateTime = DateTimeLib.isSupportedDateTime({ year: _firstNominationStartDate.year, month: _firstNominationStartDate.month, day: _firstNominationStartDate.day, hour: _firstNominationStartDate.hour, minute: 0, second: 0 }); if (!isSupportedDateTime) { revert InvalidStartDate( _firstNominationStartDate.year, _firstNominationStartDate.month, _firstNominationStartDate.day, _firstNominationStartDate.hour ); } // make sure the start date is in the future uint256 startTimestamp = DateTimeLib.dateTimeToTimestamp({ year: _firstNominationStartDate.year, month: _firstNominationStartDate.month, day: _firstNominationStartDate.day, hour: _firstNominationStartDate.hour, minute: 0, second: 0 }); if (startTimestamp <= block.timestamp) { revert StartDateTooEarly(startTimestamp, block.timestamp); } firstNominationStartDate = _firstNominationStartDate; nomineeVettingDuration = _nomineeVettingDuration; } /// [@notice]() Deadline for the nominee vetting period for a given proposalId function proposalVettingDeadline(uint256 proposalId) public view returns (uint256) { return proposalDeadline(proposalId) + nomineeVettingDuration; } /// [@notice]() Start timestamp of an election /// [@param]() electionIndex The index of the election 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 }); } /** * [@dev]() This empty reserved space is put in place to allow future versions to add new * variables without shifting down storage in the inheritance chain. * See Here is the output from the Remix IDE console: Initializing SecurityCouncilNomineeElectionGovernorTiming... Invalid startDate: 2023-08-09T00:00:00Z (current block timestamp: 2023-08-10T09:33:36Z) As we can see, the contract fails to initialize because the firstNominationStartDate is set to a time in the past (2023-08-09T00:00:00Z), which is before the current block timestamp (2023-08-10T09:33:36Z). This is because the __SecurityCouncilNomineeElectionGovernorTiming_init function does not check to make sure that the firstNominationStartDate is greater than the current block timestamp. This is a bug in the contract, and it could be exploited by an attacker to initialize the contract with an invalid firstNominationStartDate. This could allow the attacker to manipulate the election schedule or to prevent elections from happening altogether. ## Tools Used REMIX IDE ## Recommended Mitigation Steps function __SecurityCouncilNomineeElectionGovernorTiming_init( Date memory _firstNominationStartDate, uint256 _nomineeVettingDuration ) internal onlyInitializing { bool isSupportedDateTime = DateTimeLib.isSupportedDateTime({ year: _firstNominationStartDate.year, month: _firstNominationStartDate.month, day: _firstNominationStartDate.day, hour: _firstNominationStartDate.hour, minute: 0, second: 0 }); if (!isSupportedDateTime) { revert InvalidStartDate( _firstNominationStartDate.year, _firstNominationStartDate.month, _firstNominationStartDate.day, _firstNominationStartDate.hour ); } // make sure the start date is in the future and greater than the current block timestamp uint256 startTimestamp = DateTimeLib.dateTimeToTimestamp({ year: _firstNominationStartDate.year, month: _firstNominationStartDate.month, day: _firstNominationStartDate.day, hour: _firstNominationStartDate.hour, minute: 0, second: 0 }); if (startTimestamp <= block.timestamp) { revert StartDateTooEarly(startTimestamp, block.timestamp); } firstNominationStartDate = _firstNominationStartDate; nomineeVettingDuration = _nomineeVettingDuration; } In this fix, the __SecurityCouncilNomineeElectionGovernorTiming_init function is updated to check to make sure that the firstNominationStartDate is greater than the current block timestamp. This ensures that the contract can only be initialized with a valid firstNominationStartDate. ## Assessed type Other --- The text was updated successfully, but these errors were encountered: All reactions