Anubis - GovernorBravoDelegateStorage - Lack of Proper Access Control and Data Integrity

[sherlock-admin2]

Anubis

high

GovernorBravoDelegateStorage - Lack of Proper Access Control and Data Integrity

Summary

The GovernorBravoDelegateStorageV2 contract and its related storage contracts lack proper access control mechanisms for critical state variables and functions, potentially allowing unauthorized modification of governance parameters and proposal records.

Vulnerability Detail

Critical state variables such as votingDelay, votingPeriod, proposalThreshold, and mappings like proposals and latestProposalIds are public with no explicit setter functions containing access control mechanisms. This could allow unauthorized actors to modify governance settings or tamper with proposal records.

Impact

An attacker could exploit this vulnerability to disrupt the governance process by altering governance parameters or tampering with proposal records, potentially leading to incorrect governance decisions, loss of funds, or undermining the governance system's integrity.

Code Snippet

https://github.com/sherlock-audit/2024-01-olympus-on-chain-governance/blob/main/bophades/src/external/governance/abstracts/GovernorBravoStorage.sol#L91-L117

Tool used

Manual Review

Recommendation

Implement access control mechanisms such as the onlyAdmin modifier for functions that modify critical state variables or governance parameters. Ensure that state variables that hold sensitive data are either private or have controlled, restricted access.

Code Snippet for Fix:

// Use the onlyAdmin modifier for functions that should be restricted
modifier onlyAdmin {
    require(msg.sender == admin, "GovernorBravoDelegateStorage: Unauthorized");
    _;
}

function setVotingDelay(uint256 _votingDelay) public onlyAdmin {
    votingDelay = _votingDelay;
    emit VotingDelayUpdated(_votingDelay);
}

function setVotingPeriod(uint256 _votingPeriod) public onlyAdmin {
    votingPeriod = _votingPeriod;
    emit VotingPeriodUpdated(_votingPeriod);
}

function setProposalThreshold(uint256 _proposalThreshold) public onlyAdmin {
    proposalThreshold = _proposalThreshold;
    emit ProposalThresholdUpdated(_proposalThreshold);
}

// ...similar setters for other critical state variables with proper access control

By enforcing access control and ensuring the integrity of critical governance data, the contract can prevent unauthorized modifications, maintaining the governance system's integrity and intended behavior.

[nevillehuang]

Invalid, this are public GETTER functions, not SETTER functions. In GovernorBravoDelegate.sol, there are permissions in place for changing these state variables