sherlock-audit / 2024-08-flayer-judging

0 stars 0 forks source link

Attractive Ash Buffalo - nft while being transfer , transferFrom is used , which may end up losing NFT #711

Open sherlock-admin2 opened 4 days ago

sherlock-admin2 commented 4 days ago

Attractive Ash Buffalo

Medium

nft while being transfer , transferFrom is used , which may end up losing NFT

Summary

The transferFrom() method is used instead of safeTransferFrom(). I argue that this isn’t recommended because:

Vulnerability Detail

While depositing and at other places , while transfering nft from user to contract address or in opposite to msg.sender from contract address, in both ways , transfer from is used instead of safeTransferFrom. which might lock nft in any contract address which is not compatible to recieve the nft.

Impact

While depositing, Swap , SwapBatch , in all cases tranferFrom is used so , we might not be able to deposit or like this, when msg.sender would be contract which could not accept incoming ERC721 it end up losing our nfts.

Code Snippet

https://github.com/sherlock-audit/2024-08-flayer/blob/0ec252cf9ef0f3470191dcf8318f6835f5ef688c/flayer/src/contracts/Locker.sol#L226

https://github.com/sherlock-audit/2024-08-flayer/blob/0ec252cf9ef0f3470191dcf8318f6835f5ef688c/flayer/src/contracts/Locker.sol#L248C1-L252C83

Tool used

Manual Review

Recommendation

I recommend to call the safeTransferFrom() method instead of transferFrom() for NFT transfers.