increaseTotalVotingPower() can be front-ran by an attacker with a call to rageQuit() in order to withdraw more assets than the attacker should be able to claim.

[c4-submissions]

Lines of code

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

Vulnerability details

Overview of the vulnerability / PoC

---

The function increaseTotalVotingPower in PartyGovernanceNFT does not have a front-running protection against rageQuit() allowing a user to walk away with more assets than he should.

An example of the attack

1. A party member exists in a party which has a total voting power of 100e18 has a governance NFT token which has an id of 5. This governance NFT token has a voting power of 10e18 meaning that the user owns 10% share of the party.

2. An authority decides to increase the total voting power of the party (could be by minting new governance NFT cards for a user). If the totalVotingPower gets increased, that means that each member's share of the party's assets will be decreased, you can see the maths of this at https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernanceNFT.sol#L393:

    // Check token's balance.
    uint256 balance = address(withdrawTokens[i]) == ETH_ADDRESS
        ? address(this).balance
        : withdrawTokens[i].balanceOf(address(this));

    // Add fair share of tokens from the party to total.
    for (uint256 j; j < tokenIds.length; ++j) {
        // Must be retrieved before burning the token.
        withdrawAmounts[i] += (balance * getVotingPowerShareOf(tokenIds[j])) / 1e18;
    }

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

    function getVotingPowerShareOf(uint256 tokenId) public view returns (uint256) {
        uint256 totalVotingPower = _getSharedProposalStorage().governanceValues.totalVotingPower;
        return
            totalVotingPower == 0 ? 0 : (votingPowerByTokenId[tokenId] * 1e18) / totalVotingPower;
    }

3. The member will front-run the increaseTotalVotingPower call with a call to rageQuit() in PartyGovernanceNFT to to walk away with his funds.

4. The member will be able to walk away with funds amount calculated based on the old totalVotingPower which would be more than the funds the attacker would be able to claim happen he rage quits after the total voting power gets increased.

---

Impact

---

A user can claim more assets than he should claim happen the total voting power of a party gets increased by an authority for some reason.

---

Remediation

---

A potential fix is to enable a member to rage quit only after a specific period within him requesting to rage quit. Something like this:

1. User requests to rage quit
2. Smart contract asks the user to wait X amount of time
3. User requests to withdraw his funds and gets his request fulfilled.

---

Assessed type

MEV

[ydspa]

In the scope of this audit, the authority would be InitialETHCrowdfund contract, if someone tries to rageQuit() before InitialETHCrowdfund calling increaseTotalVotingPower, the one would get nothing as funds has not been sent from InitialETHCrowdfund to Paryt yet.

Invalid: Insufficient proof with in scope context

[c4-pre-sort]

ydspa marked the issue as insufficient quality report

[c4-pre-sort]

ydspa marked the issue as primary issue

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid

[gzeon-c4]

I also believe this is the intended behavior of ragequit.

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid

[gzeon-c4]

#414 (comment)