NFTBridge
60,000 USDC
View results
Submission Details
Severity: low
Invalid

Pausable collections cannot be bridged

Summary

If a pauseable NFT is bridged to Starknet, going back to Ethereum would be impossible if transfers are paused.

Vulnerability Details

Since all NFT collections are in scope, there are collections that can pause their transfers like AxieInfinity -https://etherscan.io/address/0xf5b0a3efb8e8e4c201e2a935f110eaaf3ffecb8d

When these NFTs are bridged to Startnet via Bridge::depositTokens(), the NFTs will be locked inside the Mainnet escrow and a new collection will be deployed on L2 representing AxieInfinity.

#[l1_handler]
fn withdraw_auto_from_l1(
ref self: ContractState,
from_address: felt252,
req: Request
) {
ensure_is_enabled(@self);
assert(self.bridge_l1_address.read().into() == from_address,
'Invalid L1 msg sender');
// TODO: recompute HASH to ensure data are not altered.
// TODO: Validate all fields the request (cf. FSM).
let collection_l2 = ensure_erc721_deployment(ref self, @req);

But when the user decides to transfer back his NFT from L2 to L1 and waits for his NFT to be unlocked and transferred back to him, this will not be possible as _withdrawFromEscrow will fail and thus his L2 NFT will be removed, but he you won't get it on L1, leaving it locked in the escrow contract forever.

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
if (!_isEscrowed(collection, id)) {
return false;
}
address from = address(this);
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);
} else {
// TODO:
// Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
_escrow[collection][id] = address(0x0);
return true;
}

Impact

User will lose their NFT if the original implementation of NFT pauses transfers.

Tools Used

Manual Review

Inspired by this - https://code4rena.com/reports/2024-01-renft#m-12-paused-erc721erc1155-could-cause-stoprent-to-revert-potentially-causing-issues-for-the-lender

Recommendations

It's hard to mitigate this, but either don't allow this type of collections, or on L2 provide pause as well on the collections that have it originally.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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