Party DAO - KupiaSec's results

Lines of code

https://github.com/code-423n4/2023-10-party/blob/053fb9345b0739b3c26d12e1eae1eefbfd70b223/contracts/proposals/SetGovernanceParameterProposal.sol#L25 https://github.com/code-423n4/2023-10-party/blob/053fb9345b0739b3c26d12e1eae1eefbfd70b223/contracts/party/PartyGovernance.sol#L648 https://github.com/code-423n4/2023-10-party/blob/053fb9345b0739b3c26d12e1eae1eefbfd70b223/contracts/party/PartyGovernance.sol#L1090 https://github.com/code-423n4/2023-10-party/blob/053fb9345b0739b3c26d12e1eae1eefbfd70b223/contracts/party/PartyGovernance.sol#L1105

Vulnerability details

Impact

Proposals might be executed/defeated unexpectedly with the updated governance parameters.

Proof of Concept

With the new updates, governance parameters can be changed after finalizing a party by executing the proposal in _executeSetGovernanceParameter(). It updates voteDuration, executionDelay, and passThresholdBps.

While accepting and executing proposals in accept() and _getProposalStatus(), it uses the changed params and it might bring an unexpected result.

File: 2023-10-party\contracts\party\PartyGovernance.sol
642:         // Update the proposal status if it has reached the pass threshold.
643:         if (
644:             values.passedTime == 0 &&
645:             _areVotesPassing(
646:                 values.votes,
647:                 values.totalVotingPower,
648:                 _getSharedProposalStorage().governanceValues.passThresholdBps //@audit use updated param
649:             )
650:         ) {
651:             info.values.passedTime = uint40(block.timestamp);
652:             emit ProposalPassed(proposalId);
653:             // Notify third-party platforms that the governance NFT metadata has
654:             // updated for all tokens.
655:             emit BatchMetadataUpdate(0, type(uint256).max);
656:         }

• While proposing a proposal, governanceValues.passThresholdBps = 51% and around 55% of voters have accepted it.
• Other voters liked to execute the proposal also but they didn't accept it further as it's passed already. (It's in the execution delay period).
• But a new GovernanceParameterProposal has been executed and passThresholdBps has been increased to 60%.
• After that, voters couldn't vote for the proposal due to voteDuration requirement and the proposal was defeated although most voters liked it.

Tools Used

Manual Review

Recommended Mitigation Steps

While creating a new proposal in PartyGovernance.propose(), governance parameters(voteDuration, executionDelay, and passThresholdBps) should be cached so later updates don't affect the already created proposal.

    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);
    }

Assessed type

Governance