search

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

2 stars 0 forks source link

Use `_safeMint()` instead of `_mint()` #312

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/Crowdfund.sol#L439 https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/Crowdfund.sol#L480

Vulnerability details

Impact

OpenZeppelin recommends the usage of _safeMint() instead of _mint(). If the recipient is a contract, safeMint() checks whether they can handle ERC721 tokens.

Proof of Concept

If the user provides an address that can't handle ERC721 tokens when calling contribute() to CrowdFund the minted token might be lost. That would also result in the user not being able to do any governance action (vote, delegate) on Party

contribute() callable by a user:

https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/Crowdfund.sol#L191

Resulting in the following _mint() call:

https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/Crowdfund.sol#L439

And when CrowdFund token is burnt, it called _mint() on PartyGovernance https://github.com/PartyDAO/party-contracts-c4/blob/3896577b8f0fa16cba129dc2867aba786b730c1b/contracts/crowdfund/Crowdfund.sol#L480

Tools Used

Manual Anaylsis

Recommended Mitigation Steps

Use _safeMint() whenever possible

merklejerk commented 1 year ago

Duplicate of #18

HardlyDifficult commented 1 year ago

Converting into a QA report for the warden.

HardlyDifficult commented 1 year ago

Merging with https://github.com/code-423n4/2022-09-party-findings/issues/311