NFTBridge
60,000 USDC
View results
Submission Details
Severity: low
Invalid

Lack of zero address check in Starknet `Bridge::deposit_tokens` can lead to permanent lockup of NFTs

Summary

deposit_tokens function of the L2 Bridge.cairo contract allows users to submit transactions with a zero address (0x0) for owner_l1. This can lead to permanent locking of tokens on L2 without the possibility of retrieval, creating a serious security risk.

Vulnerability Details

The deposit_tokens function in the Bridge.cairo contract on L2 allows users to deposit tokens for bridging to L1. The function accepts an owner_l1 address parameter, which specifies the intended recipient of the tokens on L1. However, there is no validation to ensure that owner_l1 is not set to the zero address (0x0).

The relevant code snippet is as follows:

fn deposit_tokens(
ref self: ContractState,
salt: felt252,
collection_l2: ContractAddress,
owner_l1: EthAddress,
token_ids: Span<u256>,
use_withdraw_auto: bool,
use_deposit_burn_auto: bool,
) {
ensure_is_enabled(@self);
assert(!self.bridge_l1_address.read().is_zero(), 'Bridge is not open');
// rest of the code..
}

Consequently, any user who sends zero-address in owner_l1 will have his request processed on L2 but cannot execute that message on L1. The zero address is considered an invalid destination address in token transfers on L1 (in the Bridge::withdrawTokens)

Upon initiating a transfer with owner_l1 set to zero:

  1. The tokens are transferred to the L2 escrow as expected.

  2. The L2 contract generates a bridging message to be consumed on L1.

  3. When the withdrawTokens function on L1 attempts to withdraw these tokens, it will likely revert due to the use of safeTransferFrom that prevent transfers to the zero address.

  4. As a result, the tokens remain locked in the L2 escrow indefinitely, without any mechanism for recovery. It is noteworthy that unlike in L1, there is no provision to cancel requests in Bridge.cairo

Impact

If the owner_l1 address is set to zero, accidentally or otherwise, tokens will be irreversibly locked in the L2 escrow contract. These tokens cannot be retrieved or refunded unless the Bridge is upgraded.

Tools Used

Manual Review

Recommendations

Consider implementing validation check in deposit_tokensto ensure owner_l1address is not set as zero.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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