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

Unchecked Replay of Withdrawal Requests

axicnode398

Description:

The withdraw_auto_from_l1 function processes messages from L1 to withdraw tokens on L2. However, there is no mechanism to prevent the same L1 message from being processed multiple times (a replay attack). If an attacker can resend an old message, they might be able to withdraw the same tokens multiple times.

Location:

withdraw_auto_from_l1 function in blockchain/starknet/src/bridge.cairo line 128, Around the lines where the request is processed and validated in the withdraw_auto_from_l1 function.

Issue:
The function does not store or check if a request has already been processed, allowing for replay attacks.

Impact:
An attacker could withdraw the same tokens multiple times, leading to loss of funds or duplicate asset withdrawals, compromising the integrity of the bridge.

Tools used: Manual Review.

Recommendations:
Implement a nonce or request hash tracking system to ensure each L1 message is processed only once.

Potential changes:
Add a mapping to store processed request hashes and modify the withdraw_auto_from_l1 function to check if a request has already been processed.

#[storage]
struct Storage {
// Existing storage variables...
​
// New: Mapping to store processed request hashes
processed_requests: LegacyMap::<felt252, bool>,
}
​
#[l1_handler]
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');
​
// New: Check if the request has been processed
assert(self.processed_requests.read(req.hash) == false, 'Request already processed');
​
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;
};
​
// New: Mark the request as processed
self.processed_requests.write(req.hash, true);
​
self.emit(WithdrawRequestCompleted {
hash: req.hash,
block_timestamp: starknet::info::get_block_timestamp(),
req_content: req
});
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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