code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Governance/TemporalGovernor.sol#L266 https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Governance/TemporalGovernor.sol#L261

Vulnerability details

Impact

It is said that TemporalGovernor.fastTrackProposalExecution Allows the guardian to process a VAA when the TemporalGovernor is paused. https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Governance/TemporalGovernor.sol#L261

    /// @notice Allow the guardian to process a VAA when the
    /// Temporal Governor is paused this is only for use during
    /// periods of emergency when the governance on moonbeam is
    /// compromised and we need to stop additional proposals from going through.
    /// @param VAA The signed Verified Action Approval to process

However, it can be executed even if the temporal governor is not paused.

Proof of Concept

TemporalGovernor.fastTrackProposalExecution Allows the guardian to process a VAA when the TemporalGovernor is paused. https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Governance/TemporalGovernor.sol#L266

    /// @notice Allow the guardian to process a VAA when the
    /// Temporal Governor is paused this is only for use during
    /// periods of emergency when the governance on moonbeam is
    /// compromised and we need to stop additional proposals from going through.
    /// @param VAA The signed Verified Action Approval to process
    function fastTrackProposalExecution(bytes memory VAA) external onlyOwner {
        _executeProposal(VAA, true); /// override timestamp checks and execute
    }

But it doesn’t check the pause status of the contract. The guardian can execute it when the temporal governor is not paused.

Tools Used

Manual Review

Recommended Mitigation Steps

Add whenPaused on fastTrackProposalExecution

-   function fastTrackProposalExecution(bytes memory VAA) external onlyOwner {
+   function fastTrackProposalExecution(bytes memory VAA) external whenPaused onlyOwner {
        _executeProposal(VAA, true); /// override timestamp checks and execute
    }

Assessed type

Access Control

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as primary issue

c4-sponsor commented 1 year ago

ElliotFriedman marked the issue as sponsor confirmed

alcueca commented 1 year ago

As I read the natspec, it says it should be usable when paused, but it doesn't say that it shouldn't be usable when not paused. However, I'll defer to the sponsor as to validity.

c4-judge commented 1 year ago

alcueca marked issue #276 as primary and marked this issue as a duplicate of 276