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

7 stars 4 forks source link

MintableIncentivizedERC721 incorrectly implements safe transfers #492

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol#L290-L301 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol#L320-L327

Vulnerability details

Impact

MintableIncentivizedERC721 incorrectly implements safeTransfer and safeTransferFrom by simply replicating the unsafe transfer/transferFrom function.

Raising as medium because as a consequence of this, these ERC721 tokens may end up locked in contracts that does not support ERC-721 tokens while at the same time offering the false impression of this event being impossible to whoever is using the safeTransfer/safeTransferFrom functions.

Proof of Concept

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol#L290-L301 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol#L320-L327

Tools Used

Recommended Mitigation Steps

Either implement safe transfer for safeTransfer/safeTransferFrom function or revert on safeTransfer/safeTransferFrom if not willing to support safe tansfers.

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