code-423n4 / 2022-11-paraspace-findings

7 stars 4 forks source link

Upgraded Q -> M from #449 [1674665297296] #518

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #449 as M risk. The relevant finding follows:

[L‑03] MintableIncentivizedERC721 does not implement ERC721.safeTransferFrom properly MintableIncentivizedERC721 is described as:

27: * @notice Basic ERC721 implementation which will be used as a parent contract for NTokens.

It however does not implement safeTransferFrom as per EIP-721. This function should:

When transfer is complete, this function /// checks if _to is a smart contract (code size > 0). If so, it calls /// onERC721Received on _to and throws if the return value is not /// bytes4(keccak256("onERC721Received(address,address,uint256,bytes)")). But looking at the implementation, it simply performs the _transfer, without performing these checks:

320: function _safeTransfer( 321: address from, 322: address to, 323: uint256 tokenId, 324: bytes memory 325: ) internal virtual { 326: _transfer(from, to, tokenId); 327: } NToken, which inherits MintableIncentivizedERC721, does not override this function. Neither do all the NToken implementations.

Recommendation Add the required check to MintableIncentivizedERC721._safeTransfer.

320: function _safeTransfer( 321: address from, 322: address to, 323: uint256 tokenId, 324: bytes memory 325: ) internal virtual { 326: _transfer(from, to, tokenId);

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #51

c4-judge commented 1 year ago

dmvt marked the issue as satisfactory