search

code-423n4 / 2022-05-cally-findings

2 stars 0 forks source link

ERC721 TRANSFERFROM() MIGHT LEAD TO USER LOSING FUNDS #277

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L258 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L318

Vulnerability details

ERC721 TRANSFERFROM() MIGHT LEAD TO USER LOSING FUNDS

In exercise() and withdraw(), the ERC721 transferFrom() method is used to transfer the premium asset from the contract to the caller. If the caller is a contract that has not implemented the onERC721Received method properly, the NFT could be locked.

Impact

Medium

Proof Of Concept

Instances include:

line 258 and line 318

Tools Used

Manual Analysis

Recommended Mitigation Steps

use OZ's safeTransferFrom() instead of TransferFrom(). This way, if someone uses a contract to buy the call option and that contract has not implemented the ERC721 Receiving method properly, the execute() function will revert.

outdoteth commented 2 years ago

use safeTransferFrom to prevent stuck NFTs: https://github.com/code-423n4/2022-05-cally-findings/issues/38