ArkProject: NFT Bridge

Summary

The depositTokens() function in Bridge.sol does not check whether the users’ provided ether (msg.value) is enough
to cover transaction costs at L1 and L2, causing user tokens to be freezed at the escrow.

Vulnerability Details

From cairo book ( https://book.cairo-lang.org/ch16-04-L1-L2-messaging.html ):

"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."

However, the msg.value is not checked in depositTokens() function in Bridge.sol before calling sendMessageToL2. This can cause the transaction to fail at L1 or L2 sides. Additionally, If there is not enough ether provided by the user for covering the fees for message to be deserialized and processed on L2, the transaction can be failed at L2 side with tokens escrowed at L1 side. Indeed, once the message is received by destination, the message is considered delivered and user's tokens will be remained at L1 escrow, even though it threw an out-of-gas error in L2.

Impact

Bridging functionality cannot proceed if there is not enough ether provided in msg.value. Also, user tokens can be freezed at escrow when there is not enough ether to cover transaction costs at L2.

It is worth mentionioning that this kind of issue was already considered as a high severity in another contest:

https://code4rena.com/reports/2024-01-decent#h-02-due-to-missing-checks-on-minimum-gas-passed-through-layerzero-executions-can-fail-on-the-destination-chain

Tools Used

Manual Review

Recommendations

depositTokens() function needs to check that enough ether is provided in msg.value to cover all the costs at L1 and L2. Based on the documentations provided in https://book.cairo-lang.org/ch16-04-L1-L2-messaging.html , the minimum required msg.value must always be 20k wei.