Digital garden

code4rena-2023-11-party-dao-m04

3 min read

code4rena-2023-11-party-dao-m04

[M-04] PartyGovernance.accept - passThresholdBps isn’t cached for each proposal which can lead to problems if changed through another proposal

보고서

Summary

제안 생성 시점의 거버넌스 설정을 캐시하지 않아, 기존 제안이 예기치 않게 업데이트된 거버넌스 설정으로 인해 통과 또는 거부될 수 있다.

Keyword

dao, logic flaw

Vulnerability

passThresholdBps 라는 변수를 통해 제안이 통과되기 위해 필요한 총 투표권 대비 투표 비율을 설정한다. 또한 이 변수는 SetGovernanceParameterProposal._executeSetGovernanceParameter을 통해 변경할 수 있다.

    /// @notice Execute a `SetGovernanceParameterProposal` which sets the given governance parameter.
    function _executeSetGovernanceParameter(
        IProposalExecutionEngine.ExecuteProposalParams memory params
    ) internal returns (bytes memory) {
        ...
        if (proposalData.passThresholdBps != 0) { // passThresholdBps can be set as well
            if (proposalData.passThresholdBps > 10000) { 
                revert InvalidGovernanceParameter(proposalData.passThresholdBps);
            }
            emit PassThresholdBpsSet(
                _getSharedProposalStorage().governanceValues.passThresholdBps,
                proposalData.passThresholdBps
            );
@>          _getSharedProposalStorage().governanceValues.passThresholdBps = proposalData
                .passThresholdBps;
        }
 
        return "";
    }

이 자체는 문제가 되지 않지만, 제안이 통과되었는지 확인하는 _areVotesPassing 함수 호출에 문제가 있다. _areVotesPassing 함수를 호출할 때, 제안 생성 당시의 passThresholdBps 값이 아니라 _getSharedProposalStorage().governanceValues.passThresholdBps 로 전역 값을 사용하고 있다.

        ...
        // Update the proposal status if it has reached the pass threshold.
        if (
            values.passedTime == 0 &&
@>          _areVotesPassing(
                values.votes,
                values.totalVotingPower,
@>              _getSharedProposalStorage().governanceValues.passThresholdBps // This isn't cached
            )
        ) {
            info.values.passedTime = uint40(block.timestamp);
            emit ProposalPassed(proposalId);
            // Notify third-party platforms that the governance NFT metadata has
            // updated for all tokens.
            emit BatchMetadataUpdate(0, type(uint256).max);
        }

Impact

업데이트된 거버넌스 설정으로 인해 제안이 예기치 않게 통과 또는 거부될 수 있다.

Mitigation

제안 생성 시점의 거버넌스 설정을 캐시하여 추후 설정이 변경되더라도 기존의 제안에는 영향이 없도록 한다.

    function propose(
        Proposal memory proposal,
        uint256 latestSnapIndex
    ) external returns (uint256 proposalId) {
        _assertActiveMember();
        proposalId = ++lastProposalId;
        // Store the time the proposal was created and the proposal hash.
        (
            _proposalStateByProposalId[proposalId].values,
            _proposalStateByProposalId[proposalId].hash
        ) = (
            ProposalStateValues({
                proposedTime: uint40(block.timestamp),
                passedTime: 0,
                executedTime: 0,
                completedTime: 0,
                votes: 0,
                totalVotingPower: _getSharedProposalStorage().governanceValues.totalVotingPower,
                numHosts: numHosts,
                numHostsAccepted: 0,
@>              voteDuration: _getSharedProposalStorage().governanceValues.voteDuration, //@audit add 3 params here
@>              executionDelay: _getSharedProposalStorage().governanceValues.executionDelay,
@>              passThresholdBps: _getSharedProposalStorage().governanceValues.passThresholdBps
            }),
            getProposalHash(proposal)
        );
        emit Proposed(proposalId, msg.sender, proposal);
        accept(proposalId, latestSnapIndex);

        // Notify third-party platforms that the governance NFT metadata has
        // updated for all tokens.
        emit BatchMetadataUpdate(0, type(uint256).max);
    }

tags: bughunting, party dao, smart contract, solidity, dao, logic flaw, severity medium

Graph View