Party DAO - rvierdiiev's results

Lines of code

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L636-L638 https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L1098-L1100

Vulnerability details

Proof of Concept

When proposal is created, then number of hosts is stored for it. In case if proposal succeeded and all hosts have accepted it, then it's considered as unanimous and execution delay(which is another point of security) is skipped. Pls, note that _hostsAccepted requires that accepted hosts amount should be strictly equal to host count that was stored in proposal.

So in case, if host accepts proposal, then count of accepted hosts is incremented.

Also PartyGovernance has abdicateHost function which allows host to move his role to another account. https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernance.sol#L457-L472

    function abdicateHost(address newPartyHost) external {
        _assertHost();
        // 0 is a special case burn address.
        if (newPartyHost != address(0)) {
            // Cannot transfer host status to an existing host.
            if (isHost[newPartyHost]) {
                revert InvalidNewHostError();
            }
            isHost[newPartyHost] = true;
        } else {
            // Burned the host status
            --numHosts;
        }
        isHost[msg.sender] = false;
        emit HostStatusTransferred(msg.sender, newPartyHost);
    }

Using this function, host can accept proposal as many times as needed to make it unanimous. He just needs to accept proposal, then abdicateHost to another his address and so on. As result, host can ability to use this function in his interests to reach needed amount of hosts needed(and skip execution delay) or even to not allow _hostsAccepted function show true, when needed amount of hosts has accepted proposal(he can make accepted hosts count be bigger, than needed).

Impact

Malicious host can use _hostsAccepted feature in his own interests.

Tools Used

VsCode

Recommended Mitigation Steps

I don't know good solution here. With current design this functionality will not work normally.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/ETHCrowdfundBase.sol#L202-L204 https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernanceNFT.sol#L195-L198

Vulnerability details

Proof of Concept

When someone is going to contribute to InitialETHCrowdfund, then _processContribution is called. Contribution can be called for yourself, or on behalf of someone.

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

        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;
        }

As you can see, in case if user already has delegate, then no one else can change it. But in case if it's not set yet, then caller can set delegator for user. This check is done only in the ETHCrowdfundBase._processContribution function and delegator inside InitialETHCrowdfund._contribute is not changed.

After that, InitialETHCrowdfund._contribute continue it's work and it provides originally passed delegator, that was provided by called to the Party.mint function.

So when PartyGovernanceNFT.mint is executed it actually fetches user's current delegate and rewrites provided one, in case if current delegate is not 0.

So later, voting power is added to the current delegator of token owner.

So what we have here is that attacker can frontrun user's first contribution(when he doesn't have any delegate yet) with minimum contribution and delegator that he would like to have. So then, when user's contribution will be executed, then his delegator will be ignored and the one, provided by attacker before will be used. As result, attacker was able to increase his voting power.

Impact

Attacker can steal user's voting power.

Tools Used

VsCode

Recommended Mitigation Steps

In this case we can't understand if user has agreed to use provided delegate. Maybe better to use offchain signatures for such cases. In case if someone wants to contribute on behalf of someone, then he should provide user's signature.

Or allow third party to delegate only for current delegator of owner. In case if it's first nft of user, then delegator should be user itself.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/InitialETHCrowdfund.sol#L297-L298

Vulnerability details

Proof of Concept

Using InitialETHCrowdfund._contribute, user can provide some funds together with delegate and receive some voting power instead and delegate it(together with all other user's voting power) to provided delegate.

It's possible that user will provide 0 amount. As comment states it can be used to change user's delegator. The problem is that this will change delegator only inside ETHCrowdfundBase, which is actually useless for the Party and then function will return. And real delegator in the Party will not be changed in this case.

As result, user will think that he had changed delegator, but in reality it will be same.

Impact

Delegator for user will not be changed.

Tools Used

VsCode

Recommended Mitigation Steps

In case if amount is 0, then change delegator for user.

if (amount == 0) {
    delegateVotingPower(delegate);
    return;
}

But be careful with using this by third parties. This should not be allowed or somehow restricted.

Assessed type

Error