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

Users unable to bridge because `escrow` map on L2 is never updated

Summary

The bridge on L2 does not update the escrow map when withdrawing NFTs causing users' NFTs to be stuck in the bridge when using auto-burn.

Vulnerability Details

Currently when an NFT is bridged, it is escrowed in the bridge from which it comes. If the user specifies the auto-burn flag, this NFT should be burnt after successful bridging.
In order to keep track of escrowed NFTs, both bridges (L1 and L2) have mappings indicating if a NFT is escrowed or not.
On L2 this is done like this:

fn withdraw_auto_from_l1(
ref self: ContractState,
from_address: felt252,
req: Request
) {
// [...]
let mut i = 0;
loop {
if i == req.ids.len() {
break ();
}
let token_id = *req.ids[i];
let to = req.owner_l2;
let from = starknet::get_contract_address();
let is_escrowed = !self.escrow.read((collection_l2, token_id)).is_zero();
if is_escrowed {
IERC721Dispatcher { contract_address: collection_l2 }
.transfer_from(from, to, token_id);
} else {
if (req.uris.len() != 0) {
let token_uri = req.uris[i];
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge_uri(to, token_id, token_uri.clone());
} else {
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge(to, token_id);
}
}
i += 1;
};
self.emit(WithdrawRequestCompleted {
hash: req.hash,
block_timestamp: starknet::info::get_block_timestamp(),
req_content: req
});
}

We see that if the collection's token_id is contained in the escrowed mapping, the bridge tries to transfer the NFT to the user as it assumes that it is escrowed in the bridge.
The problem is that the escrow mapping is never updated. This means that once the NFT is marked as escrowed, it will always be, even if the NFT is transferred out of the bridge!

Impact and PoC

Let's take the following scenario:

  • User bridges NFT from L2 to L1 -> NFT is escrowed in L2 bridge

  • User bridges NFT back from L1 to L2 with ENABLED auto-burn

    • -> NFT gets burnt on L1

    • -> NFT gets transferred to owner on L2

  • User bridges the NFT back to L1 with ANOTHER bridge which burns it

  • User bridges back to L2

  • Now the bridge thinks the NFT is still escrowed in it even though it is not existent on L2

  • This means withdraw_auto_from_l1 will fail, causing the bridging to fail needing admin intervention to get the NFT back again.

Tools Used

Manual review

Recommended Mitigation

This is easy to fix by updating the escrow mapping, removing the NFT if the transfer was successful. (As it is done on L1)

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

finding-L2-withdrawing-do-not-clean-escrow-mapping

Impact: Incorrect state without any other impact, which deserves a Low according to CodeHawks documentation.

Appeal created

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-L2-withdrawing-do-not-clean-escrow-mapping

Impact: Incorrect state without any other impact, which deserves a Low according to CodeHawks documentation.

Support

FAQs

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