ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Tokens at risk of being locked in L2 recipient contracts that do not support ERC721 Receiver Interface

hydrapenta

Summary

The withdraw_auto_from_l1 function in the L2 Bridge.cairo contract uses the transfer_from function to transfer tokens from the escrow to the recipient. This differs from the _withdrawFromEscrow function in the L1 Escrow.sol contract, which uses safeTransferFrom. This inconsistency may lead to a situation where tokens are sent to recipient contracts that do not implement the required onERC721Received interface, potentially causing tokens to become permanently locked in those contracts.

Vulnerability Details

In the L2 Bridge.cairo contract, the withdraw_auto_from_l1 function uses the transfer_from function to transfer tokens from escrow to the recipient:

fn withdraw_auto_from_l1(
ref self: ContractState,
from: ContractAddress,
to: ContractAddress,
token_id: u256,
) {
// Transfer tokens without additional checks
ERC721::transfer_from(from, to, token_id);
}
​

This approach does not perform any checks to ensure that the to address is capable of receiving ERC721 tokens, such as checking for the implementation of the onERC721Received interface. In contrast, the _withdrawFromEscrow function in the L1 Escrow.sol contract uses safeTransferFrom:

function _withdrawFromEscrow(
CollectionType ctype,
address collectionL1,
address ownerL1,
uint256 tokenId
) internal returns (bool) {
// Use safeTransferFrom to ensure that the recipient can receive ERC721 tokens
IERC721(collectionL1).safeTransferFrom(address(this), ownerL1, tokenId);
return true;
}
​

The safeTransferFrom function ensures that the recipient is either an externally owned account (EOA) or a contract that implements the onERC721Received interface, as shown in the OpenZeppelin implementation:

function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual {
transferFrom(from, to, tokenId);
ERC721Utils.checkOnERC721Received(_msgSender(), from, to, tokenId, data);
}

If the recipient contract does not implement this interface, the transaction reverts, preventing the loss of tokens. The use of transfer_from in withdraw_auto_from_l1 bypasses this safeguard, leading to potential loss of tokens if the recipient contract does not support the onERC721Received interface.

Impact

Tokens may be sent to recipient contracts that do not support the onERC721Received interface, leading to tokens being locked in the recipient contract without any way to retrieve them.

Considering that Arkis a NFT Bridge, it is likely that there are 100's of transactions involving contracts and the onus is on the protocol to ensure that no transfers lead to token lockup scenarios.

Tools Used

Manual review

Recommendations

Consider modifying** **the withdraw_auto_from_l1 function to use safe_transfer_from instead of transfer_from to ensure that the recipient can properly receive the ERC721 tokens.

Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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