Buying non-ERC721 NFTs is not supported

[code423n4]

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/BuyCrowdfundBase.sol#L128-L139

Vulnerability details

Impact

The _buy function of the BuyCrowdfundBase contract can call any function of any contract to buy any NFT (like cryptopunks), but since cryptopunks does not have the ownerOf function, this prevents the contract from buying non-ERC721 standard NFTs like cryptopunks even if it raises enough Eth.

        {
            // Execute the call to buy the NFT.
            (bool s, bytes memory r) = callTarget.call{ value: callValue }(callData);
            if (!s) {
                r.rawRevert();
            }
        }
        // Make sure we acquired the NFT we want.
        if (token.safeOwnerOf(tokenId) != address(this)) {
            revert FailedToBuyNFTError(token, tokenId);
        }

And the AuctionCrowdfund contract does not support bidding for non-ERC721 standard NFTs.

Proof of Concept

https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/BuyCrowdfundBase.sol#L128-L139

Tools Used

None

Recommended Mitigation Steps

Consider filtering for non-ERC721 compliant NFTs when creating BuyCrowdfund/CollectionBuyCrowdfund/AuctionCrowdfund contracts.
Or add support for non-ERC721-compliant NFTs

[merklejerk]

Duplicate of #248

[HardlyDifficult]

This is a NC feature suggestion, converting into a QA report for the warden.

[HardlyDifficult]

Merging with #143