code-423n4 / 2023-01-astaria-findings

5 stars 2 forks source link

Use safeMint for ERC721 #581

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L588

Vulnerability details

Impact

Users may lose their NFTs if _mint is not implemented properly.

Proof of Concept

In CollateralToken.sol, the parameter from_ is used to mint the ERC721 Collateral token to.

  _mint(from_, collateralId);

If from_ is a contract address that does not support ERC721, the Collateral token can 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.

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

As per the documentation of ERC721.sol by OpenZeppelin:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L274-L285

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L588

Tools Used

Manual Review

Recommended Mitigation Steps

use safeMint instead of mint to check received address support for ERC721 implementation.

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #83

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

Picodes marked the issue as grade-c