search

code-423n4 / 2023-06-llama-findings

2 stars 1 forks source link

Using transferFrom on ERC721 tokens transfer #245

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-llama/blob/main/src/accounts/LlamaAccount.sol#L200

Vulnerability details

Impact

The transferFrom() method is used instead of safeTransferFrom which is not recommended. It can result in loss of NFT if the address is not able to handle the received NFT. OpenZeppelin’s documentation discourages the use of transferFrom(), use safeTransferFrom() whenever possible.

Given that any NFT can be used for the call option, there are a few NFTs that have logic in the onERC721Received() function, which is only triggered in the safeTransferFrom() function and not in transferFrom().

Proof of Concept

https://github.com/code-423n4/2023-06-llama/blob/main/src/accounts/LlamaAccount.sol#L200

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using safeTransferFrom over transferFrom when exercising. safeTransferFrom ensures address is capable of handling ERC721 NFTs.

Assessed type

ERC721

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #36

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid