sherlock-audit / 2023-10-mzero-judging

3 stars 2 forks source link

dany.armstrong90 - The probability that proposers receive `Proposal Fee` may be reduced. #50

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

dany.armstrong90

high

The probability that proposers receive Proposal Fee may be reduced.

Summary

When proposers propose Standard Proposal, they pay Proposal Fee to the protocols. If this Proposal is executed, the proposer will be rewarded again. However, if StandardGovernor is Reset, the probability of returning fee is reduced by not having vote for the proposals in the Pending and Active.

Vulnerability Detail

The StandardGovernor.sol#propose function is as follows.

    function propose(
        address[] memory targets_,
        uint256[] memory values_,
        bytes[] memory callDatas_,
        string memory description_
    ) external override returns (uint256 proposalId_) {
        uint256 voteStart_;

        (proposalId_, voteStart_) = _propose(targets_, values_, callDatas_, description_);

        // If this is the first proposal for the `voteStart_` epoch, inflate its target total supply of `PowerToken`.
        if (++numberOfProposalsAt[voteStart_] == 1) {
            IPowerToken(voteToken).markNextVotingEpochAsActive();
        }

        uint256 proposalFee_ = proposalFee;

        if (proposalFee_ == 0) return proposalId_;

        address cashToken_ = cashToken;

        _proposalFees[proposalId_] = ProposalFeeInfo({ cashToken: cashToken_, fee: proposalFee_ });

L167:   if (!ERC20Helper.transferFrom(cashToken_, msg.sender, address(this), proposalFee_)) revert TransferFromFailed();
    }

As shown in the above L167 the proposers must pay Proposal Fee when they propose. And it is rewarded when proposal is executed as shown in following L139.

    function execute(
        address[] memory,
        uint256[] memory,
        bytes[] memory callDatas_,
        bytes32
    ) external payable returns (uint256 proposalId_) {
        // Proposals have voteStart=N and voteEnd=N, and can be executed only during epoch N+1.
        uint16 latestPossibleVoteStart_ = _clock() - 1;

        proposalId_ = _tryExecute(callDatas_[0], latestPossibleVoteStart_, latestPossibleVoteStart_);

        ProposalFeeInfo storage proposalFeeInfo_ = _proposalFees[proposalId_];
        uint256 proposalFee_ = proposalFeeInfo_.fee;
        address cashToken_ = proposalFeeInfo_.cashToken;

        if (proposalFee_ > 0) {
            delete _proposalFees[proposalId_];
L139:       _transfer(cashToken_, _proposals[proposalId_].proposer, proposalFee_);
        }
    }

The ZeroGovernor.sol#_deployEphemeralContracts function is as follows.

    function _deployEphemeralContracts(
        address emergencyGovernorDeployer_,
        address powerTokenDeployer_,
        address standardGovernorDeployer_,
        address bootstrapToken_,
        address cashToken_,
        uint16 emergencyProposalThresholdRatio_,
        uint256 proposalFee_
    ) internal returns (address standardGovernor_, address emergencyGovernor_, address powerToken_) {
        address expectedPowerToken_ = IPowerTokenDeployer(powerTokenDeployer_).nextDeploy();
        address expectedStandardGovernor_ = IStandardGovernorDeployer(standardGovernorDeployer_).nextDeploy();

        emergencyGovernor_ = IEmergencyGovernorDeployer(emergencyGovernorDeployer_).deploy(
            expectedPowerToken_,
            expectedStandardGovernor_,
            emergencyProposalThresholdRatio_
        );

        standardGovernor_ = IStandardGovernorDeployer(standardGovernorDeployer_).deploy(
            expectedPowerToken_,
            emergencyGovernor_,
            cashToken_,
            proposalFee_,
            _MAX_TOTAL_ZERO_REWARD_PER_ACTIVE_EPOCH
        );

        if (expectedStandardGovernor_ != standardGovernor_) {
            revert UnexpectedStandardGovernorDeployed(expectedPowerToken_, powerToken_);
        }

        powerToken_ = IPowerTokenDeployer(powerTokenDeployer_).deploy(bootstrapToken_, standardGovernor_, cashToken_);

        if (expectedPowerToken_ != powerToken_) revert UnexpectedPowerTokenDeployed(expectedPowerToken_, powerToken_);
    }

As we see in the above ZeroGovernor.sol#_deployEphemeralContracts function, when deploying the new StandardGovernor, there is no processing for Proposal Fee. The status of Proposal used in TTG includes Pending, Active, Defeated, Succeeded, Expired. If the state is Expired or Defeated, fee is sent to DistributionVault using the StandardGovernor.sol #sendProposalFeeToVault function. And if the state is Succeeded, you can execute and return the fee to the proposer.

On the other hand, because PowerToken has been updated, the old StandardGovernor.sol cannot perform Cast vote because of the access control modifiers. Since voting is not available, the processing of Proposal' in thePendingandActive` states is not done properly, and fee distribution is not done properly.

Impact

The fee distribution is not exactly done.

Code Snippet

https://github.com/sherlock-audit/2023-10-mzero/blob/main/ttg/src/ZeroGovernor.sol#L162

Tool used

Manual Review

Recommendation

When Reset Proposal is executed, we handle fee for proposals in the Pending and Active states.

Duplicate of #53