Vulnerability Details

When Bridging NFTs from L1 to L2, Starknet sequencer is responsible for calling withdraw_auto_from_l1 to complete the Bridging process.

The problem is that unlike L2 -> L1 case where the user himself is the one who withdraws his tokens, and in the case of reverting he can re-execute the tx again by giving correct values. The Sequencer will do that transaction withdraw_auto_from_l1 only one time and if it gets reverted it will not process it again, nor we will be able to call withdraw_auto_from_l1 again.

The problem here is that we are depending on 100% success of that transaction and we are not taking into considerations that it can revert.

Once the sequencer accepts the tx and updated L1 state it calls Starknet::processMessages(), which resets that message hash into 0, and by doing this we will not be able to cancel it via Starknet::startL1ToL2MessageCancellation().

StarknetCore:
Output.sol#L151

StarknetCore:
StarknetMessaging.sol#L156

Now we want to ask a question, can this transaction gets reverted?
The answer is yes maybe, although it should work correctly but what we are doing is not easy. the function logic is complex we can deploy new Collections, Verify addresses, store whitelisting, etc...

bridge.cairo#L128-L133

I grouped the possible things that will lead to reverting the tx.

1. Bridge Should Be enabled: Bridge should always be enabled, but enabled is reset to false either by the admin or when upgrading, so if the L2Bridge upgraded, or the admin disabled both of the Bridged, the on-fly tx that was executed on L1 and still in verification process on Starknet will get reverted.

2. Bridge address should be correct: this should not be a problem.

3. Deploying the Collection if needed: there are a lot of actions and verifications we are doing at this point

   1. verify_collection_address: this function ensures the correct linking between NFT collection on L1/L2, this should always succeed in most of the cases but there may be some errors, or upgrading (as the Admins can change L1L2 mapping).

   2. deploy_erc721_bridgeable: We are deploying new collections if needed, and we cannot guarantee that the deploying process will not revert.

4. Collection type should be valid: this should always success.

5. transferring tokens if existed: this is an important thing we want to take into consideration, transferring the NFT can revert because of different reasons, either NFT does not exist, the NFT collection implements a pausing mechanism, the receiver is not able to receive it, etc...

6. Minting Bridged token: this should always succeed under normal circumstances.

7. The Sequencer tx itself can revert: We need to keep in mind that the sequencer caps the gas to a certain value to protect himself from paying more gas than the amount paid in L1, and because white_listing an NFT collection tx cost increases with the increase of number of whitelisted collection (as we are iterating over all values and put it in the last), if in the verification time, some NFT collections created. this will increase the gas cost of the TX calculated by the sequencer and paid by the user, which can make the TX go OOG (as the sequencer caps the gas to the amount paid by the user), Which opens up the possibility of reverting because of OOG error in some situations.

So we can conclude that the transaction can revert some reasons can lie under admin issues, others are likely impossible to occur, and others are possible to occur. The issue will result in a total loss of NFTs on L1 Bridge and there is no way ro recover them back.

Proof of Concept

• UserA Bridged some NFTs from L1 -> L2

• Starknet Sequencer validated the transaction and tried to execute it on L2

• The transaction reverted (because of one of the following reasons)

• User NFTs lost forever

Impact

Permenant Loss of NFTs

Recommendations

The simple mitigation for this is to implement a function to transfer escrowed NFTs on L1 back to the user, this function should be authorized by the protocol.

The other solution which is a decentralized one but is complex, is to implement a verification method for such a thing. Something in my mind is that we can store the Request hash in contract storage, and if this issue occurs, we compare the Hash with the Hash on StarknetCore. if it exists in our storage and its value is zero in StarknetCore, besides the tokens are still escrowed, we give them to the user. But implementing this thing should be handled carefully to not fall into more critical issues.