ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

`escrow` mapping is not set to 0 after NFT transfer in `withdraw_auto_from_l1()` after NFT transfer in `bridge.cairo`

sammy

Summary

escrow mapping is not set to 0 after NFT transfer in withdraw_auto_from_l1() after NFT transfer in bridge.cairo

Vulnerability Details

The escrow mapping in bridge.cairo tracks whether an NFT is escrowed in the bridge contract is not. However, this mapping is not set to zero when the NFT is transferred out of the contract in withdraw_auto_from_l1(). This may cause confusion as the escrow mapping will be active even though the contract doesn't hold the particular NFT. In rare cases, it may also cause a revert.

fn withdraw_auto_from_l1(
ref self: ContractState,
from_address: felt252,
req: Request
) {
ensure_is_enabled(@self);
assert(self.bridge_l1_address.read().into() == from_address,
'Invalid L1 msg sender');
​
// TODO: recompute HASH to ensure data are not altered.
// TODO: Validate all fields the request (cf. FSM).
​
let collection_l2 = ensure_erc721_deployment(ref self, @req);
​
let _ctype = collection_type_from_header(req.header);
// TODO: check CollectionType to support ERC1155 + metadata.
​
let mut i = 0;
loop {
if i == req.ids.len() {
break ();
}
​
let token_id = *req.ids[i];
​
let to = req.owner_l2;
let from = starknet::get_contract_address();
​
let is_escrowed = !self.escrow.read((collection_l2, token_id)).is_zero();
​
if is_escrowed {
@=> IERC721Dispatcher { contract_address: collection_l2 }
.transfer_from(from, to, token_id);
} else {
if (req.uris.len() != 0) {
let token_uri = req.uris[i];
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge_uri(to, token_id, token_uri.clone());
} else {
IERC721BridgeableDispatcher { contract_address: collection_l2 }
.mint_from_bridge(to, token_id);
}
}
​
i += 1;
};
​
self.emit(WithdrawRequestCompleted {
hash: req.hash,
block_timestamp: starknet::info::get_block_timestamp(),
req_content: req
});
}

Impact

Low

Tools Used

Manual Review

Recommendations

Set escrow mapping to 0 after NFT transfer.

Updates

Lead Judging Commences

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

finding-L2-withdrawing-do-not-clean-escrow-mapping

Impact: Incorrect state without any other impact, which deserves a Low according to CodeHawks documentation.

Appeal created

n0kto Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-L2-withdrawing-do-not-clean-escrow-mapping

Impact: Incorrect state without any other impact, which deserves a Low according to CodeHawks documentation.

Support

FAQs

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