code423n4 commented 2 years ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/crowdfund/Crowdfund.sol#L301

Vulnerability details

Impact

In the function _createParty of contract Crowdfund.sol, when transferring the acquired NFTs to the new party, the transferFrom function is called instead of safeTransferFrom. If the address(party_) is a contract address that doesn't support ERC721 tokens , the NFT could be frozen in the contract

As per the documentation of EIP-721:

A wallet/broker/auction application MUST implement the wallet interface if it will accept safe transfers.

Ref: https://eips.ethereum.org/EIPS/eip-721

Proof of Concept

In line https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/crowdfund/Crowdfund.sol#L301,

preciousTokens[i].transferFrom(address(this), address(party_), preciousTokenIds[i]);

Tools Used

Manual review

Recommended Mitigation Steps

Consider changing transferFrom to safeTransferFrom at line 301.

merklejerk commented 1 year ago

Duplicate of #20

HardlyDifficult commented 1 year ago

The rec for this line is invalid.

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