sach1r0 - For ERC721 transfers, use `safeTransferFrom()` instead of `transferFrom()`

[github-actions[bot]]

sach1r0

medium

For ERC721 transfers, use safeTransferFrom() instead of transferFrom()

Summary

The _initiateBridgeERC721 function uses the transferFrom() method instead of safeTransferFrom() when transferring ERC721.

Vulnerability Detail

Instead of using safeTransferFrom(), the _initiateBridgeERC721 function uses the transferFrom() method. But the documentation of OpenZeppelin discourages the usage of transferFrom(), instead use safeTransferFrom() wherever possible.

Impact

If the recipient isn't capable of receiving ERC721 then there NFTs may be permanently lost.

Code Snippet

https://github.com/ethereum-optimism/optimism/blob/3f4b3c328153a8aa03611158b6984d624b17c1d9/packages/contracts-bedrock/contracts/L1/L1ERC721Bridge.sol#L101

Tool used

Manual Review

Recommendation

Call the safeTransferFrom() method instead of transferFrom() for NFT transfers.

[rcstanciu]

Comment from Optimism

---

Description: Use safeTransferFrom() instead of transferFrom() for outgoing erc721 transfers

Reason: This is an incoming not outgoing transfer, and we know that the Bridge is able to accept ERC721s