Lines of code
<https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/Governance/TemporalGovernor.sol#L295-L342&gt;

Vulnerability details

Impact

in the _queueProposal function there is no check for if the requested time is passed to allow queue the proposal. in this case any proposal after creating can be added to the queue list.

Proof of Concept

the TemporalGovernor.sol contract have a variable that is used to return the amount of time a proposal need to wait before going into the process(execute or queue):

 /// @notice returns the amount of time a proposal must wait before being processed.
    uint256 public immutable proposalDelay;

but this variable is not used to check if the time is passed to process a proposal in queue function:

function _queueProposal(bytes memory VAA) private {
        //@audit we don't check if the proposal delay is passed or not
        // This call accepts single VAAs and headless VAAs
        (
            IWormhole.VM memory vm,
            bool valid,
            string memory reason
        ) = wormholeBridge.parseAndVerifyVM(VAA);

        // Ensure VAA parsing verification succeeded.
        require(valid, reason);

        address intendedRecipient;
        address[] memory targets; /// contracts to call
        uint256[] memory values; /// native token amount to send
        bytes[] memory calldatas; /// calldata to send

        (intendedRecipient, targets, values, calldatas) = abi.decode(
            vm.payload,
            (address, address[], uint256[], bytes[])
        );

        _sanityCheckPayload(targets, values, calldatas);

        // Very important to check to make sure that the VAA we're processing is specifically designed
        // to be sent to this contract
        require(
            intendedRecipient == address(this),
            "TemporalGovernor: Incorrect destination"
        );

        // Ensure the emitterAddress of this VAA is a trusted address
        require(
            trustedSenders[vm.emitterChainId].contains(vm.emitterAddress), /// allow multiple per chainid
            "TemporalGovernor: Invalid Emitter Address"
        );

        /// Check that the VAA hasn't already been processed (replay protection)
        require(
            queuedTransactions[vm.hash].queueTime == 0,
            "TemporalGovernor: Message already queued"
        );

        /// Effect

        // Add the VAA to queued messages so that it can't be replayed
        queuedTransactions[vm.hash].queueTime = block.timestamp.toUint248();

        emit QueuedTransaction(intendedRecipient, targets, values, calldatas);
    }

in this case every proposal that is created can be set to the queue list by users (the function is PERMISSIONLESS) before the requested time has passed or not:

 /// @notice We explicitly don't care who is relaying this, as long
    /// as the VAA is only processed once AND, critically, intended for this contract.
    /// @param VAA The signed Verified Action Approval to process
    /// @dev callable only when unpaused
    function queueProposal(bytes memory VAA) external whenNotPaused {
        _queueProposal(VAA);
    }

Tools Used

manual review

Recommended Mitigation Steps

recommend to add check if the block.timeStamp is more than or equal to the proposalDelay

Assessed type

Governance

---

The text was updated successfully, but these errors were encountered:

All reactions