Use safeTransferFrom() function instead of transferFrom()

[code423n4]

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/PartyGovernanceNFT.sol#L136-L144

Vulnerability details

Impact

The transferFrom() method is used in the contract PartyGovernanceNFT.sol, I however argue that this isn’t recommended because: OpenZeppelin’s documentation discourages the use of transferFrom(), use safeTransferFrom() whenever possible https://docs.openzeppelin.com/contracts/4.x/api/token/erc721#IERC721-transferFrom-address-address-uint256- Given that any NFT can be used for the call option, there are a few NFTs that have logic in the onERC721Received() function, which is only triggered in the safeTransferFrom() function and not in transferFrom()

Also transferFrom() function is followed by safeTransferFrom() function in the contract, it makes no sense to keep it.

Proof of Concept

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/party/PartyGovernanceNFT.sol#L136-L144

Tools Used

Visual Inspection

Recommended Mitigation Steps

Delete the transferFrom() function in the contract PartyGovernanceNFT.sol

[merklejerk]

The line in question is simply the ERC721 implementation of transferFrom(). If we removed it, we would break spec.