code-423n4 / 2022-09-party-findings

2 stars 0 forks source link

Canceling proposal breaks party #343

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/party/PartyGovernance.sol#L712

Vulnerability details

Impact

can leave a party broken locking all or any assets

Recommended Mitigation Steps

Trasnfer any/all funds to another party contract escentially setup a new party before taking the last resort of canceling a proposal that could potentially leave a party in a broken state, by putting this in place, if some how is less experienced carries out this activity there is at least some portection in place, as it is stated this should be a last resort only, propper precautions should be put into place to protect any parties that may need to do this.

it does not seem to be good practice to allow a function that can potentially break a party without having some way of protecting any assets that may be held, it could most certainly loose trust in the project by its users.

/// @dev proposal.cancelDelay seconds must have passed since it was first /// executed for this to be valid. /// The currently active proposal will simply be yeeted out of existence /// so another proposal can execute. /// This is intended to be a last resort and can leave the party /// in a broken state. Whenever possible, active proposals should be /// allowed to complete their lifecycle

merklejerk commented 1 year ago

This is valid criticism but also a known limitation we called out earlier and have accepted for this iteration. As mentioned, cancels are a last resort.

HardlyDifficult commented 1 year ago

This report is just restating the risk from comments, but the rec is a potential improvement suggestion. Merging with #347