The users may front-run the authorities by calling rageQuit()

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

In PartyGovernanceNFT.sol, there is a function called decreaseVotingPower() and it allows to decrease the votingPower of a certain tokenId. It can only be called by the authorities. The problem is that the rageQuit() function can be called by anyone who holds the tokenId with votingPower and the users may avoid decreasing their votingPower if they see the authorities's transaction to do so. The transaction, in its turn, will revert due to underflow as the votingPower that the tokenId holds will be updated after burning the tokenId.

Proof of Concept

1. The authorities want to decrease somebody's votingPower and they call decreaseVotingPower() function.
2. The owner of the tokenId sees the tx and immediately front-runs it with calling rageQuit() so his tokenId will be burnt and the money will be withdrawn:

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

 uint96 totalVotingPowerBurned = _burnAndUpdateVotingPower(tokenIds, !isAuthority_);

3. The tx for decreasing voting power is reverted due to underflow as the user's balance is now updated:

https://github.com/code-423n4/2023-10-party/blob/main/contracts/party/PartyGovernanceNFT.sol#L236-242

 function decreaseVotingPower(uint256 tokenId, uint96 votingPower) external {
        _assertAuthority();
        mintedVotingPower -= votingPower;
        votingPowerByTokenId[tokenId] -= votingPower;

        _adjustVotingPower(ownerOf(tokenId), -votingPower.safeCastUint96ToInt192(), address(0));
    }

Tools Used

Manual review.

Recommended Mitigation Steps

Transaction to decreaseVotingPower() should be executed via private mempool.

Assessed type

Other

[ydspa]

authority typically a smart contract(InitialETHCrowdfund in this audit scope) is trusted role, expected to behavior correctly

Invalid: Insufficient proof with in scope context

[c4-pre-sort]

ydspa marked the issue as insufficient quality report

[gzeon-c4]

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/README.md?plain=1#L93

• Authorities have “root user privileges” over a Party and is it well aware that they can exploit or break Parties in a wide variety of ways, especially if they are set to a malicious EOA or unsafe smart contract. This is why authorities are almost always smart contracts that are highly audited and trusted, and are NOT expected to be EOAs.

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid

[rodiontr]

I think this issue is the same as #414 and it has nothing to do with authorities privileges as rageQuit() can be called by anyone. Would like to hear your opinion on this one

Moreover, maybe issue #170 could be also somehow connected to these ones as i don't think it's a duplicate of #489

[c4-judge]

gzeon-c4 marked the issue as duplicate of #545

[rodiontr]

@gzeon-c4 sorry it seems like issue #100 has the same attack vector and should be dup

[gzeon-c4]

#414 (comment)