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

Users bridging a native L1 collection to L2 can't bridge it back leading to their tokens being stuck

0xasen

Summary

Users bridging a native L1 collection to L2 can't bridge it back leading to their tokens being stuck permanently in the bridge.

Vulnerability Details

Let's have the scenario where an L1 collection is deployed on L1, L2 collection is not yet deployed and we're bridging L1 -> L2 for the 1st time.

We call depositTokens, the request is created and let's see the value of this:

req.collectionL1 = collectionL1;
req.collectionL2 = _l1ToL2Addresses[collectionL1];

req.collectionL1 will be the address of the collection on L1 and req.collectionL2 will be 0 since the collection is yet to be deployed on L2 and the mapping is empty.

Now the message is sent and withdraw_auto_from_l1 is called on L2. We go to this line:

let collection_l2 = ensure_erc721_deployment(ref self, @req);

and let's see the code of the ensure_erc721_deployment function:

fn ensure_erc721_deployment(ref self: ContractState, req: @Request) -> ContractAddress {
​
//extract the L1 and L2 collection addresses from the request
let l1_req: EthAddress = *req.collection_l1;
let l2_req: ContractAddress = *req.collection_l2;
​
//verify if the collection address is valid and deployed on L2
let collection_l2 = verify_collection_address(
l1_req,
l2_req,
self.l2_to_l1_addresses.read(l2_req),
self.l1_to_l2_addresses.read(l1_req),
);
​
//if the collection is deployed, return its address
if !collection_l2.is_zero() {
return collection_l2;
}
​
//generate a salt for the deployment using the request hash
let hash = *req.hash;
let salt_data: Span<felt252> = array![hash.low.into(), hash.high.into()].span();
let salt = poseidon_hash_span(salt_data);
​
//deploy the collection
let l2_addr_from_deploy = deploy_erc721_bridgeable(
self.erc721_bridgeable_class.read(),
salt,
req.name.clone(),
req.symbol.clone(),
req.base_uri.clone(),
starknet::get_contract_address(),
);
​
//update the mappings between L1 and L2 collection addresses
self.l1_to_l2_addresses.write(l1_req, l2_addr_from_deploy);
self.l2_to_l1_addresses.write(l2_addr_from_deploy, l1_req);
​
//whitelist functionality below

let collection_l2 = verify_collection_address will return 0 since the collection is yet to be deployed, we'll proceed with let l2_addr_from_deploy = deploy_erc721_bridgeable which deploys the function and the mappings on the L2 contract will be updated:

//update the mappings between L1 and L2 collection addresses
self.l1_to_l2_addresses.write(l1_req, l2_addr_from_deploy);
self.l2_to_l1_addresses.write(l2_addr_from_deploy, l1_req);

Now, let's try and bridge back the same token from the collection to L1.

We call deposit_tokens, where collection_l2 is the address of the collection on L2, let collection_l1 = self.l2_to_l1_addresses.read(collection_l2); returns the address of the collection on L1 since that mapping has been updated.

Message is processed, we call withdrawTokens on L1:

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
if (!_enabled) {
revert BridgeNotEnabledError();
}
​
uint256 header = request[0];
​
if (Protocol.canUseWithdrawAuto(header)) {
revert NotSupportedYetError();
} else {
_consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request);
}
​
Request memory req = Protocol.requestDeserialize(request, 0);
​
//Verifies the mapping between the request addresses and the storage.
address collectionL1 = _verifyRequestAddresses(req.collectionL1, req.collectionL2);
​
//rest of the functionality
}

Let's focus on this line address collectionL1 = _verifyRequestAddresses(req.collectionL1, req.collectionL2);, where req.collectionL1 and req.collectionL2 are the addresses of the collection on L1 and L2 accordingly as they are passed by the L2 contract which has the updated mappings as we confirmed.

​
function _verifyRequestAddresses(
address collectionL1Req,
snaddress collectionL2Req
)
internal
view
returns (address)
{
address l1Req = collectionL1Req;
uint256 l2Req = snaddress.unwrap(collectionL2Req);
​
//retrieve the L1 address mapped to the provided L2 address from storage
address l1Mapping = _l2ToL1Addresses[collectionL2Req];
//retrieve the L2 address mapped to the provided L1 address from storage
uint256 l2Mapping = snaddress.unwrap(_l1ToL2Addresses[l1Req]);
​
//check if only L2 address is present in the request and L1 address is not
if (l2Req > 0 && l1Req == address(0)) {
//functionality if only one of the addresses is present in the request
}
​
//check if both L2 and L1 addresses are provided in the request
if (l2Req > 0 && l1Req > address(0)) {
if (l1Mapping != l1Req) {
revert InvalidCollectionL1Address();
} else if (l2Mapping != l2Req) {
revert InvalidCollectionL2Address();
​
} else {
return l1Mapping;
}
}
​
revert ErrorVerifyingAddressMapping();
}

We enter into this if (l2Req > 0 && l1Req > address(0)) since both l2Req and l1Req are not 0, and we go into the following.

if (l1Mapping != l1Req) {
revert InvalidCollectionL1Address();
}

However, address l1Mapping = _l2ToL1Addresses[collectionL2Req]; is address(0) since that mapping was never updated, and l1Req is not 0 as we already pointed out, so the function enters the if and reverts.

Now, the users' token on the L2 is in the escrow and he can't claim it back, and he can't withdraw his token on L1 leading to his NFTs being stuck forever.

Impact

Once you bridge a token from a native L1 collection to L2 you can't bridge it back leading to your tokens being stuck.

Tools Used

Manual review

Recommendations

In the case where both l2Req and l1Req are present just return l1Req since that value is passed from the L2 bridge contract and can be trusted.

Updates

Lead Judging Commences

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

finding-first-bridgeof-a-collection-L1<->L2-do-not-sync-addresses

Likelyhood: High, any collections bridged, without bridge owner action, will be unable to bridge back. Impact: High, L2 -> L1 tokens will be stuck in the bridge. L1 -> L2 will need to ask for a cancellation.

Support

FAQs

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