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

Escrow is not updated when withdrawing on L2

Summary

During withdrawal on L2, the escrow is not updated to ContractAddressZeroable::zero().

Vulnerability Details

During withdrawal on L2, it should update the escrow to ContractAddressZeroable::zero() same as the approach done on L1:

bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L201

_escrow[collection][id] = address(0x0);

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Escrow.sol#L86

But, on L2, this is missing. This leads to a situation that although the token is withdrawn on L2, the escrow returns a non-zero address, incorrectly meaning that the NFT is still escrowed.

Impact

  • Wrong state value.

  • Missing updating a state variable.

Tools Used

Recommendations

Following line should be added:

let is_escrowed = !self.escrow.read((collection_l2, token_id)).is_zero();
if is_escrowed {
+ self.escrow.write((collection_l2, token_id), ContractAddressZeroable::zero());
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);
}
}

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/starknet/src/bridge.cairo#L157

Updates

Lead Judging Commences

n0kto Lead Judge 10 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 9 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.