Users may accidentally send ETH to `Party.sol` and `PartyGovernance.sol` without any way to retrieve the funds back

[code423n4]

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/Party.sol#L47 https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/PartyGovernance.sol#L314-L319

Vulnerability details

Description

The Party.sol contract is responsible for initialisation of the PartyGovernanceNFT which is to be delegateCalled by the Proxy constructor. At the end of the contract there's a receive() payable function which can accept ETH into the contract directly.

In the case of PartyGovernance.sol, there exists a fallback function for the purpose of facilitating eip-1271 signatures (note that the PartyGovernance.sol uses the TokenDistributor to distribute tokens for users.

In both cases ETH shouldn't be sent to these contracts.

This was rated as "Medium" in severity because of the likelihood that users might accidentally send ETH to these contracts thinking that they were for the purpose of contributions is negligible.

Impact

Negligible users could successfully send ETH to the contracts with no way of getting their funds back.

Proof of Concept

I've included two proof of concepts for each contract demonstrating the accidental transfer of ETH for Party.sol and PartyGovernance.sol.

Tools Used

Manual code inspection

Recommended Mitigation Steps

For Party.sol I recommend the following actions:

• Remove the receive function if possible or check if the msg.sender was a contract.

For PartyGovernance.sol:

• I recommend checking if msg.value is > 0. If so revert the transaction.

[merklejerk]

This isn't a supported interaction.

[HardlyDifficult]

This is a valid consideration to help protect against user error. Downgrading to QA and merging with #250