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

NFTs bridged from L2's can be lost forever due to inexistent ownerL1 address checks

Summary

NFTs can be lost forever if deposit_tokens is called with ownerL1 set as the zero address or 0xdEaD address, making the NFT unaccessible on L1 and the deposited L2 nft's unretrievable by normal methods.

Vulnerability Details

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,)

The code above allows bridging of NFTs from Starknet to Ethereum, and the Bridged NFT owner on L1 is specified by the owner_l1 variable, but the contract fails to prevent setting the owner address to the 0 or dead address which would mean the NFT can be lost forever due to errors. Likelihood is low, but it should be prevented anyways.

Tools Used

Manual Review

Recommendations

Add the following line to the fn deposit_tokens

let zero_address = EthAddress::from(0);
let dead_address = EthAddress::from(0x000000000000000000000000000000000000dEaD);
assert(owner_l1 != zero_address && owner_l1 != dead_address, 'owner_l1 cannot be the zero or dead address');
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year 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.