NFTBridge
60,000 USDC
View results
Submission Details
Severity: medium
Valid

If L1 receiver has no `onERC721Received` implemention, the bridge from L2 to L1 will revert and NFT will be stuck in L1 bridge contract

Summary

If L1 receiver has no onERC721Received implemention, the bridge from L2 to L1 will revert and NFT will be stuck in L1 bridge contract.

Vulnerability Details

Bridge::withdrawTokens

bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);

Escrow::_withdrawFromEscrow

if (!_isEscrowed(collection, id)) {
return false;
}
address from = address(this);
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);

When bridging from L2 to L1 and NFT is escrowed on L1, the NFT is transfered by calling safeTransferFrom. If the L1 receiver(req.ownerL1) is smart contract and has no onERC721Received implemention,withdrawTokens will revert.

Impact

L1 receiver won't get NFT. And NFT will be stuck in L1 bridge contract.

Tools Used

manual

Recommendations

if (collectionType == CollectionType.ERC721) {
- IERC721(collection).safeTransferFrom(from, to, id);
+ IERC721(collection).transferFrom(from, to, id);
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-withdraw-safeTransferFrom-to-no-onERC721Received-will-revert

Impact: High, NFT will be stuck in L2 bridge. Likelyhood: Very low, sending NFT to a contract not implementing that function would almost be a user error.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!