code423n4 commented 2 years ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/PartyGovernance.sol#L1066

Vulnerability details

Impact

Currently, unanimously voted proposals allow them to bypass executionDelay in addition to unlocking specific behaviours in the post voting stage. The conditions for a "unanimously voted" proposal is 99.99% of the total voting power in the party.

Consider established communities who decide to start a party which may have a large following on social media. As parties grow in size, the tolerance for the losslessness may increase which could mean that proposals which are deemed "unanimous", would not be considered as such thus, preventing the extra perks that unanimously voted proposals might get.

I've awarded this as a "medium" in severity because whilst there is no direct attack in play here, unanimously voted proposals are not given the perks they deserve, going against the business logic of the application. In addition to this, the likelihood of this happening is unlikely since governance parties would need to be of a sizeable amount of participants.

Proof of Concept

Source: https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/PartyGovernance.sol#L1066

    function _isUnanimousVotes(uint96 totalVotes, uint96 totalVotingPower)
        private
        pure
        returns (bool)
    {
        uint256 acceptanceRatio = (totalVotes * 1e4) / totalVotingPower;
        // If >= 99.99% acceptance, consider it unanimous.
        // The minting formula for voting power is a bit lossy, so we check
        // for slightly less than 100%.
        return acceptanceRatio >= 0.9999e4;
    }

The protocol will except only %99.99 of the total voting power which could be arbitrary and is also quite lossy.

Tools Used

Manual code inspection

Recommended Mitigation Steps

I recommend allowing users who form a party to create a custom value (perhaps in the ranges of %97.5 - %99.99) to allow for a bit of additional leeway at the discretion of the party creator(s).

merklejerk commented 1 year ago

OK with this limitation for now and may address it in the future. We think it's unlikely to be an issue in most parties. It's probable that very large parties would often have a difficult time reaching even 90% consensus anyway.

HardlyDifficult commented 1 year ago

It's an interesting suggestion. Since there's no attack here - downgrading and merging with the warden's QA report #250

code-423n4 / 2022-09-party-findings