Contributor cannot update the delegate in crowdfunding contribution

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-party/blob/06e6271b75a30f00ae2674e8c26f3eb9e89e75e5/contracts/crowdfund/ETHCrowdfundBase.sol#L201-L208
https://github.com/code-423n4/2023-10-party/blob/06e6271b75a30f00ae2674e8c26f3eb9e89e75e5/contracts/crowdfund/InitialETHCrowdfund.sol#L300-L302
https://github.com/code-423n4/2023-10-party/blob/06e6271b75a30f00ae2674e8c26f3eb9e89e75e5/contracts/party/PartyGovernanceNFT.sol#L194-L198

Vulnerability details

Impact

A contributor's attempt to update their delegate during a contribution does not work as intended, and the contributor may allow the outdated delegate to use contributor's voting power.

Proof of Concept

When in initial crowdfund, the system permits delegate updates only if the contributor is the message sender or if no prior delegation exists. It updates the delegationsByContributor state variable.

    function _processContribution(
        address payable contributor,
        address delegate,
        uint96 amount
    ) internal returns (uint96 votingPower) {
        address oldDelegate = delegationsByContributor[contributor];
        if (msg.sender == contributor || oldDelegate == address(0)) {
            // Update delegate.
            delegationsByContributor[contributor] = delegate;
        } else {
            // Prevent changing another's delegate if already delegated.
            delegate = oldDelegate;
        }

https://github.com/code-423n4/2023-10-party/blob/06e6271b75a30f00ae2674e8c26f3eb9e89e75e5/contracts/crowdfund/ETHCrowdfundBase.sol#L201-L208

However, there is an existed delegate, the contributor cannot update it to a new one. It is because the new delegate will be an argument used to mint a new party card.

    if (tokenId == 0) {
        // Mint contributor a new party card.
        party.mint(contributor, votingPower, delegate); 

https://github.com/code-423n4/2023-10-party/blob/06e6271b75a30f00ae2674e8c26f3eb9e89e75e5/contracts/crowdfund/InitialETHCrowdfund.sol#L300-L302

In PartyGovernanceNFT.mint function, it will not use the delegate from the argument but reverts to the delegate address from delegatesByVoter which is the old delegate.

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

https://github.com/code-423n4/2023-10-party/blob/06e6271b75a30f00ae2674e8c26f3eb9e89e75e5/contracts/party/PartyGovernanceNFT.sol#L194-L198

It essentially denies the update of delegate in initial crowdfund contribution.

Tools Used

Manual

Recommended Mitigation Steps

Check if the minting is in initial phase, update the delegate to a new one in mint function.

Assessed type

Other

[c4-pre-sort]

ydspa marked the issue as duplicate of #334

[c4-pre-sort]

ydspa marked the issue as insufficient quality report

[c4-judge]

gzeon-c4 marked the issue as duplicate of #418

[c4-judge]

gzeon-c4 marked the issue as not a duplicate

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid

[thangtranth]

hi @gzeon-c4,

I would like to ask for this issue being re-reviewed. The report shows that the contributor cannot change the delegate. The delegate is updated in ETHCrowdfundBase but not in PartyGovernanceNFT. I think it should be a medium issue. Can you give me the reason why is it invalid?

Thank you.

[gzeon-c4]

This failed to described the front-running attack. The delegate issue if not malicious can be fixed with calling delegateVotingPower.