ArkProject: NFT Bridge

Summary

Vulnerability Detail

The Starklane contract serves as a bridge between Ethereum and Starknet, allowing users to transfer tokens between the two networks. The depositTokens() function is a critical component of this bridge, enabling users to deposit tokens into escrow and initiate a transfer to Starknet.

The function takes several parameters:

• salt: A value used to generate a unique request hash

• collectionL1: Address of the token collection contract on Ethereum

• ownerL2: Address of the new owner on Starknet

• ids: Array of token IDs to be transferred

• useAutoBurn: A flag to determine if tokens should be automatically burned

The function generates a request hash using these parameters and processes the deposit. However, a critical vulnerability exists in the implementation: the request hash is not stored or checked against previous requests. This oversight allows for potential replay attacks.

The root cause of this issue lies in the following code section:

As indicated by the TODO comment, the contract fails to implement a crucial security measure: storing and checking the request hash to prevent replay attacks.

Impact

This issue allows an attacker to replay deposit transactions, potentially draining funds from the bridge. By repeatedly submitting the same deposit request, an attacker could transfer the same tokens multiple times, creating discrepancies between the actual token balances and the bridge's recorded state.

Proof of Concept

1. Alice initiates a legitimate deposit of 10 valuable NFTs by calling depositTokens() with a specific set of parameters.

2. The bridge processes Alice's request, moving her NFTs into escrow and initiating the transfer to Starknet.

3. An attacker observes this transaction and captures the parameters used.

4. The attacker calls depositTokens() with the exact same parameters used in Alice's transaction.

5. Due to the lack of request hash verification, the bridge processes this replayed request as if it were a new, legitimate deposit.

6. The attacker repeats step 4 multiple times, each time causing the bridge to record a new deposit of Alice's NFTs without actually moving any additional tokens.

7. On the Starknet side, multiple deposit events are recorded for the same NFTs, potentially allowing the attacker to withdraw more NFTs than were originally deposited.

Tools Used

Manual review

Recommendation

To mitigate this vulnerability, implement a system to track and verify request hashes. This can be achieved by adding a mapping to store processed request hashes and checking against this mapping before processing any deposit request.

Here's a suggested fix: