code-423n4 / 2023-05-party-findings

1 stars 1 forks source link

Rage quitting availability cannot be reliably guaranteed #29

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-party/blob/f6f80dde81d86e397ba4f3dedb561e23d58ec884/contracts/party/PartyGovernanceNFT.sol#L298-L307 https://github.com/code-423n4/2023-05-party/blob/f6f80dde81d86e397ba4f3dedb561e23d58ec884/contracts/party/PartyGovernanceNFT.sol#L270

Vulnerability details

Impact

The host can block specific rage quits, invalidating some of the security offered by the rage quit functionality.

Proof of Concept

Rage quitting is only allowed before rageQuitTimestamp or if permanently enabled:

// Check if ragequit is allowed.
uint40 currentRageQuitTimestamp = rageQuitTimestamp;
if (currentRageQuitTimestamp != ENABLE_RAGEQUIT_PERMANENTLY) {
    if (
        currentRageQuitTimestamp == DISABLE_RAGEQUIT_PERMANENTLY ||
        currentRageQuitTimestamp < block.timestamp
    ) {
        revert CannotRageQuitError(currentRageQuitTimestamp);
    }
}

However, the host can change rageQuitTimestamp at will. This enables the host to frontrun a rage quitter with a resetting of rageQuitTimestamp, reverting his rage quit. Therefore a user can never be certain that he can rage quit. The user might have entered the party with the expectation, or even on condition, that he has the choice to rage quit (for the time being). There must be some guarantee that this functionality is available when it says it is. It also seems wise to be able to rage quit against a misbehaving host.

Recommended Mitigation Steps

Let the new rageQuitTimestamp come into effect only after a certain delay. At a minimum a block to prevent the frontrunning scenario described above, a few blocks to take the possibility of block stuffing into account, or some period of time to allow for human reaction time.

Assessed type

Context

0xble commented 1 year ago

Hosts are suppose to have a lot of agency and authority within a party and this was by design. The mitigation hurts the hosts ability to respond to emergencies, which is what rage quit was originally motivated by.

c4-sponsor commented 1 year ago

0xble marked the issue as sponsor acknowledged

c4-judge commented 1 year ago

thereksfour marked the issue as primary issue

c4-judge commented 1 year ago

thereksfour marked the issue as duplicate of #16

c4-judge commented 1 year ago

thereksfour marked the issue as satisfactory