Party Protocol - Invitational Contest

Contest Details

• Total Prize Pool: $17,050 USDC
  • HM awards: $11,178 USDC
  • QA awards: $1,315 USDC
  • Gas awards: $657 USDC
  • Judge awards: $3,400 USDC
  • Scout awards: $500 USDC
• Join C4 Discord to register
• Submit findings using the C4 form
• Read our guidelines for more details
• Starts May 26, 2023 20:00 UTC
• Ends May 30, 2023 20:00 UTC

Automated Findings / Publicly Known Issues

Automated findings output for the audit can be found here within 24 hours of audit opening.

Publicly Known Issues

There are some potential risks with this feature that are known possibilities. These are not considered vulnerabilities and are described below:

1. Ragequitting too many tokens at once might run out of gas and fail. The end user can only ragequit a limited number of tokens.
2. If a user intentionally or accidentally excludes a token in their ragequit, they forfeit that token and will not be able to claim it.
3. After a user rage quits, the votes delegated to them will not be undone. Those who delegated to them will need to redelegate their votes.

Note for C4 wardens: Anything included in the automated findings output is considered a publicly known issue and is ineligible for awards.

Overview

Party Protocol aims to be a standard for group coordination, providing on-chain functionality for essential group behaviors:

1. Formation: Assembling a group and combining resources.

2. Coordination: Making decisions and taking action together.

3. Distribution: Sharing resources with group members.

The first version of the Party Protocol had a focus on NFTs. The next release will expand the protocol, introducing the new concept of parties that can hold and use ETH to do anything.

The new type of party in this next release is something we've referred to as "ETH parties," to distinguish from previous "NFT parties." ETH parties have fewer restrictions and guardrails than NFT parties did. This is intentional, since it allows ETH parties much more flexibility and freedom. However, this also means that malicious proposals are more of a concern. If a malicious contributor is able to obtain a high amount of voting power in a Party such that they can pass proposals on their own, they could pass a proposal to drain a Party of its assets. Party Hosts already have a veto power to block malicious proposals, but we decided to add additional protections against this type of attack.

To protect members from the threat of a 51% attack (or any attack where a single actor has enough votes to pass a proposal), we have added a "rage quit" ability like that of MolochDAO's. If a malicious proposal were to pass, rage quit would allow everyone else in the party to quit during the execution delay, each taking their share of the party's fungible assets with them.

Rage quit can be enabled or disabled by a Party Host and can be changed at any time, unless it is locked to ENABLE_RAGEQUIT_PERMENANTLY or DISABLE_RAGEQUIT_PERMENANTLY. To offer more flexibility than just on or off, rage quit is implemented as a timestamp until which rage quit is enabled. This supports a wider set of party configurations. For example, a Party Host could enable rage quit for a limited time, after which it would be disabled again.

The rage quit feature is to the protocol, and we want to make sure it is implemented correctly. We are looking for a security audit of the rage quit functionality, particularly its interaction with governance.

Documentation

For more information on Party Protocol, see the documentation here.

• Overview
• Crowdfund
• Governance
• Prior C4 Audit Repo

Quickstart Command

Here's a one-liner to immediately get started with the codebase. It will clone the project, build it, run every test, and display gas reports:

export ETH_RPC_URL='<your_alchemy_mainnet_url_here>' && rm -Rf 2023-05-party || true && git clone https://github.com/code-423n4/2023-05-party -j8 --recurse-submodules && cd 2023-05-party && foundryup && forge install && yarn install && forge test -f $ETH_RPC_URL --gas-report

Scope

• PartyGovernanceNFT (new functions setRageQuit() and rageQuit())
• PartyGovernance (new variable lastBurnTimestamp)
• In addition to code in these new functions, any vulnerabilities or unintended effects of these functions related to voting, proposals, or the party's assets is in scope.

Differences from Last C4 Contest

The changes from post-mitigation code of the last C4 contest and the current one.

diff --git a/contracts/party/PartyGovernance.sol b/contracts/party/PartyGovernance.sol
index 3b2d2b2..b062e06 100644
--- a/contracts/party/PartyGovernance.sol
+++ b/contracts/party/PartyGovernance.sol
@@ -184,6 +184,7 @@ abstract contract PartyGovernance is
     error InvalidBpsError(uint16 bps);
     error DistributionsRequireVoteError();
     error PartyNotStartedError();
+    error CannotRageQuitAndAcceptError();

     uint256 private constant UINT40_HIGH_BIT = 1 << 39;
     uint96 private constant VETO_VALUE = type(uint96).max;
@@ -198,6 +199,8 @@ abstract contract PartyGovernance is
     uint16 public feeBps;
     /// @notice Distribution fee recipient.
     address payable public feeRecipient;
+    /// @notice The timestamp of the last time `burn()` was called.
+    uint40 public lastBurnTimestamp;
     /// @notice The hash of the list of precious NFTs guarded by the party.
     bytes32 public preciousListHash;
     /// @notice The last proposal ID that was used. 0 means no proposals have been made.
@@ -583,6 +586,17 @@ abstract contract PartyGovernance is
             }
         }

+        // Prevent voting in the same block as the last burn timestamp.
+        // This is to prevent an exploit where a member can burn their card to
+        // reduce the total voting power of the party, then propose and vote in
+        // the same block since `getVotingPowerAt()` uses `values.proposedTime - 1`.
+        // This would allow them to use the voting power snapshot just before
+        // their card was burned to vote, potentially passing a proposal that
+        // would have otherwise not passed.
+        if (lastBurnTimestamp == block.timestamp) {
+            revert CannotRageQuitAndAcceptError();
+        }
+
         // Cannot vote twice.
         if (info.hasVoted[msg.sender]) {
             revert AlreadyVotedError(msg.sender);
diff --git a/contracts/party/PartyGovernanceNFT.sol b/contracts/party/PartyGovernanceNFT.sol
index c8de1f3..10aecc2 100644
--- a/contracts/party/PartyGovernanceNFT.sol
+++ b/contracts/party/PartyGovernanceNFT.sol
@@ -14,28 +15,48 @@ import "../renderers/RendererStorage.sol";
 contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
     using LibSafeCast for uint256;
     using LibSafeCast for uint96;
+    using LibERC20Compat for IERC20;
+    using LibAddress for address payable;

     error OnlyAuthorityError();
     error OnlySelfError();
     error UnauthorizedToBurnError();
+    error FixedRageQuitTimestampError(uint40 rageQuitTimestamp);
+    error CannotRageQuitError(uint40 rageQuitTimestamp);
+    error CannotDisableRageQuitAfterInitializationError();
+    error InvalidTokenOrderError();

     event AuthorityAdded(address indexed authority);
     event AuthorityRemoved(address indexed authority);
+    event RageQuitSet(uint40 oldRageQuitTimestamp, uint40 newRageQuitTimestamp);
+    event RageQuit(uint256[] indexed tokenIds, IERC20[] withdrawTokens, address receiver);
+
+    uint40 private constant ENABLE_RAGEQUIT_PERMANENTLY = 0x6b5b567bfe; // uint40(uint256(keccak256("ENABLE_RAGEQUIT_PERMANENTLY")))
+    uint40 private constant DISABLE_RAGEQUIT_PERMANENTLY = 0xab2cb21860; // uint40(uint256(keccak256("DISABLE_RAGEQUIT_PERMANENTLY")))
+
+    // Token address used to indicate ETH.
+    address private constant ETH_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

     // The `Globals` contract storing global configuration values. This contract
-    // is immutable and it’s address will never change.
+    // is immutable and its address will never change.
     IGlobals private immutable _GLOBALS;

-    /// @notice Address with authority to mint cards and update voting power for the party.
-    mapping(address => bool) public isAuthority;
     /// @notice The number of tokens that have been minted.
     uint96 public tokenCount;
     /// @notice The total minted voting power.
     ///         Capped to `_governanceValues.totalVotingPower` unless minting
     ///         party cards for initial crowdfund.
     uint96 public mintedVotingPower;
+    /// @notice The timestamp until which ragequit is enabled. Can be set to the
+    ///         `ENABLE_RAGEQUIT_PERMANENTLY`/`DISABLE_RAGEQUIT_PERMANENTLY`
+    ///         values to enable/disable ragequit permanently.
+    ///         `DISABLE_RAGEQUIT_PERMANENTLY` can only be set during
+    ///         initialization.
+    uint40 public rageQuitTimestamp;
     /// @notice The voting power of `tokenId`.
     mapping(uint256 => uint256) public votingPowerByTokenId;
+    /// @notice Address with authority to mint cards and update voting power for the party.
+    mapping(address => bool) public isAuthority;

     modifier onlyAuthority() {
         if (!isAuthority[msg.sender]) {
@@ -51,9 +72,9 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
         _;
     }

-    // Set the `Globals` contract. The name of symbol of ERC721 does not matter;
+    // Set the `Globals` contract. The name or symbol of ERC721 does not matter;
     // it will be set in `_initialize()`.
-    constructor(IGlobals globals) PartyGovernance(globals) ERC721("", "") {
+    constructor(IGlobals globals) payable PartyGovernance(globals) ERC721("", "") {
         _GLOBALS = globals;
     }

@@ -66,7 +87,8 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
         ProposalStorage.ProposalEngineOpts memory proposalEngineOpts,
         IERC721[] memory preciousTokens,
         uint256[] memory preciousTokenIds,
-        address[] memory authorities
+        address[] memory authorities,
+        uint40 rageQuitTimestamp_
     ) internal {
         PartyGovernance._initialize(
             governanceOpts,
@@ -76,8 +98,11 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
         );
         name = name_;
         symbol = symbol_;
-        for (uint256 i; i < authorities.length; ++i) {
-            isAuthority[authorities[i]] = true;
+        rageQuitTimestamp = rageQuitTimestamp_;
+        unchecked {
+            for (uint256 i; i < authorities.length; ++i) {
+                isAuthority[authorities[i]] = true;
+            }
         }
         if (customizationPresetId != 0) {
             RendererStorage(_GLOBALS.getAddress(LibGlobals.GLOBAL_RENDERER_STORAGE))
@@ -122,7 +147,7 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
     }

     /// @inheritdoc ITokenDistributorParty
-    function getDistributionShareOf(uint256 tokenId) external view returns (uint256) {
+    function getDistributionShareOf(uint256 tokenId) public view returns (uint256) {
         uint256 totalVotingPower = _governanceValues.totalVotingPower;

         if (totalVotingPower == 0) {
@@ -143,8 +168,9 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
         uint256 votingPower,
         address delegate
     ) external onlyAuthority onlyDelegateCall returns (uint256 tokenId) {
-        (uint96 tokenCount_, uint96 mintedVotingPower_) = (tokenCount, mintedVotingPower);
+        uint96 mintedVotingPower_ = mintedVotingPower;
         uint96 totalVotingPower = _governanceValues.totalVotingPower;
+
         // Cap voting power to remaining unminted voting power supply.
         uint96 votingPower_ = votingPower.safeCastUint256ToUint96();
         // Allow minting past total voting power if minting party cards for
@@ -154,7 +180,9 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
         }

         // Update state.
-        tokenId = tokenCount = tokenCount_ + 1;
+        unchecked {
+            tokenId = ++tokenCount;
+        }
         mintedVotingPower += votingPower_;
         votingPowerByTokenId[tokenId] = votingPower_;

@@ -202,9 +230,9 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
         _governanceValues.totalVotingPower += newVotingPower;
     }

-    /// @notice Burn a NFT and remove its voting power.
+    /// @notice Burn a governance NFT and remove its voting power.
     /// @param tokenId The ID of the NFT to burn.
-    function burn(uint256 tokenId) external onlyDelegateCall {
+    function burn(uint256 tokenId) public onlyDelegateCall {
         address owner = ownerOf(tokenId);
         uint96 totalVotingPower = _governanceValues.totalVotingPower;
         bool authority = isAuthority[msg.sender];
@@ -219,6 +247,9 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
             if (totalVotingPower != 0 || !authority) revert UnauthorizedToBurnError();
         }

+        // Update last burn timestamp.
+        lastBurnTimestamp = uint40(block.timestamp);
+
         uint96 votingPower = votingPowerByTokenId[tokenId].safeCastUint256ToUint96();
         mintedVotingPower -= votingPower;
         delete votingPowerByTokenId[tokenId];
@@ -234,6 +265,93 @@ contract PartyGovernanceNFT is PartyGovernance, ERC721, IERC2981 {
         _burn(tokenId);
     }

+    /// @notice Set the timestamp until which ragequit is enabled.
+    /// @param newRageQuitTimestamp The new ragequit timestamp.
+    function setRageQuit(uint40 newRageQuitTimestamp) external onlyHost {
+        uint40 oldRageQuitTimestamp = rageQuitTimestamp;
+
+        // Prevent disabling ragequit after initialization.
+        if (newRageQuitTimestamp == DISABLE_RAGEQUIT_PERMANENTLY) {
+            revert CannotDisableRageQuitAfterInitializationError();
+        }
+
+        // Prevent setting timestamp if it is permanently enabled/disabled.
+        if (
+            oldRageQuitTimestamp == ENABLE_RAGEQUIT_PERMANENTLY ||
+            oldRageQuitTimestamp == DISABLE_RAGEQUIT_PERMANENTLY
+        ) {
+            revert FixedRageQuitTimestampError(oldRageQuitTimestamp);
+        }
+
+        emit RageQuitSet(oldRageQuitTimestamp, rageQuitTimestamp = newRageQuitTimestamp);
+    }
+
+    /// @notice Burn a governance NFT and withdraw a fair share of fungible tokens from the party.
+    /// @param tokenIds The IDs of the governance NFTs to burn.
+    /// @param withdrawTokens The fungible tokens to withdraw.
+    /// @param receiver The address to receive the withdrawn tokens.
+    function rageQuit(
+        uint256[] calldata tokenIds,
+        IERC20[] calldata withdrawTokens,
+        address receiver
+    ) external {
+        // Check if ragequit is allowed.
+        uint40 currentRageQuitTimestamp = rageQuitTimestamp;
+        if (currentRageQuitTimestamp != ENABLE_RAGEQUIT_PERMANENTLY) {
+            if (
+                currentRageQuitTimestamp == DISABLE_RAGEQUIT_PERMANENTLY ||
+                currentRageQuitTimestamp < block.timestamp
+            ) {
+                revert CannotRageQuitError(currentRageQuitTimestamp);
+            }
+        }
+
+        // Used as a reentrancy guard. Will be updated back after ragequit.
+        delete rageQuitTimestamp;
+
+        for (uint256 i; i < tokenIds.length; ++i) {
+            uint256 tokenId = tokenIds[i];
+
+            // Must be retrieved before burning the token.
+            uint256 shareOfVotingPower = getDistributionShareOf(tokenId);
+
+            // Burn caller's party card. This will revert if caller is not the owner
+            // of the card.
+            burn(tokenId);
+
+            // Withdraw fair share of tokens from the party.
+            IERC20 prevToken;
+            for (uint256 j; j < withdrawTokens.length; ++j) {
+                IERC20 token = withdrawTokens[j];
+
+                // Prevent null and duplicate transfers.
+                if (prevToken >= token) revert InvalidTokenOrderError();
+
+                prevToken = token;
+
+                // Check if token is ETH.
+                if (address(token) == ETH_ADDRESS) {
+                    // Transfer fair share of ETH to receiver.
+                    uint256 amount = (address(this).balance * shareOfVotingPower) / 1e18;
+                    if (amount != 0) {
+                        payable(receiver).transferEth(amount);
+                    }
+                } else {
+                    // Transfer fair share of tokens to receiver.
+                    uint256 amount = (token.balanceOf(address(this)) * shareOfVotingPower) / 1e18;
+                    if (amount != 0) {
+                        token.compatTransfer(receiver, amount);
+                    }
+                }
+            }
+        }
+
+        // Update ragequit timestamp back to before.
+        rageQuitTimestamp = currentRageQuitTimestamp;
+
+        emit RageQuit(tokenIds, withdrawTokens, receiver);
+    }
+
     /// @inheritdoc ERC721
     function transferFrom(
         address owner,

Files in scope

All other source contracts (not in scope)

Scoping Details

- If you have a public code repo, please share it here: There's a private internal PR covering the new features, but the protocol is here: https://github.com/PartyDAO/party-protocol
- How many contracts are in scope?: 2
- Total SLoC for these contracts?: 77
- How many external imports are there?: 0
- How many separate interfaces and struct definitions are there for the contracts within scope?: 0
- Does most of your code generally use composition or inheritance?: inheritance
- How many external calls?: 0
- What is the overall line coverage percentage provided by your tests?: 80
- Is there a need to understand a separate part of the codebase / get context in order to audit this part of the protocol?: true
- Please describe required context: Understanding how party cards are minted and how governance works and how voting power is updated and snapshotted will help with understanding the full scope of what happens when a user rage quits from the party.
- Does it use an oracle?: no
- Does the token conform to the ERC20 standard?: no
- Are there any novel or unique curve logic or mathematical models?: no
- Does it use a timelock function?: no
- Is it an NFT?: true
- Does it have an AMM?: false
- Is it a fork of a popular project?: false
- Does it use rollups?: false
- Is it multi-chain?: false
- Does it use a side-chain?: false

Slither Issue

Note that slither does not seem to be working with the repo as-is 🤷, resulting in an enum type not found error:

slither.solc_parsing.exceptions.ParsingError: Type not found enum Crowdfund.CrowdfundLifecycle