ArkProject: NFT Bridge

Summary

When bridiging NFT from L1 to Starknet, user use the depositTokens function. This function is payable because starknet sequencer expect some fees to use as gas for calling the function annotated with #[l1_handler] on the starknet side. However the contract have no check in place to ensure that user sends enough msg.value to cover:

1. The registration of the hash of the message in the storage of ethereum.

2. since the L1HandlerTransaction executed by the sequencer is not tied to any account (the message originates from L1), you must also ensure that you pay enough fees on L1 for your message to be deserialized and processed on L2.

Vulnerability Details

Consider this scenario:

1. Alice want to bridge one or many nft to starknet, so she call depositTokens() on ethereum with the appropriate params.

2. Alice send the transaction with just enough msg.value worth of ETH to cover the storage of the message hash on ethereum .

3. The transaction suceed on L1 and when the sequencer try to call the #[l1_handler] transaction on starknet it runs out of gas and the NFT gets stuck inside the SnCore contract.

4. See the ref: https://book.cairo-lang.org/ch16-04-L1-L2-messaging.html

I have coded this PoC, just copy and paste it inside Bridge.t.sol and then run it using:

forge test --mt test_NFTStuckInBridge -vv

Impact

User NFT will get stuck inside the SnCore contract

Tools Used

Manual review

Recommendations

According to cairo-book: Use Snforge or Starkli to get an estimate of the cost of the message execution then enforce that users msg.value >= to the cost. The final cost will be the cost of the SnMessaging to store the hash on etehreum + The cost of the L1Handler tx on Starknet.

It's important to note that we have {value: msg.value}. In fact, the minimum value we've to send here is 20k wei, due to the fact that the StarknetMessaging contract will register the hash of our message in the storage of Ethereum.

In addition to those 20k wei, since the L1HandlerTransaction executed by the sequencer is not tied to any account (the message originates from L1), you must also ensure that you pay enough fees on L1 for your message to be deserialized and processed on L2.

The fees of the L1HandlerTransaction are computed in a regular manner as it would be done for an Invoke transaction. For this, you can profile the gas consumption using starkli or snforge to estimate the cost of your message execution.