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

Missing syscall result check during depositing on L2 leads to loss of NFT in case Starknet is down temporarily

fyamf

Summary

The syscall results is not checked during depositing a Token on L2. So, in case of unresponsive starknet, the escrowed NFT on L2 will not be bridged to L1, and would become unreachable.

Vulnerability Details

Suppose bridging from L2 to L1 is not possible due to any reason, like: Starknet is down temporarily (as it happened recently https://status.starknet.io/incidents/d5c155c60fxs), or due to a bug/issue, the bridging is paused or inexecutable.

In this case, if a user deposits an NFT on L2 to be bridged to L1, the NFT would be escrowed on L2, but when send_message_to_l1_syscall is called, it would be unsuccessful. Since, the result of this system call is not checked, the deposit_tokens transaction would be executed, but in reality no message is bridged to L1. As a result, the user's NFT would be escrowed on L2 without being bridged, and impossible to be withdrawn on L2.

starknet::send_message_to_l1_syscall(
self.bridge_l1_address.read().into(),
buf.span(),
)
.unwrap_syscall();

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

Please note that during deploying a contract on L2, the system call result is checked, while during sending a message to L1, the system call result check is missing.

match starknet::deploy_syscall(class_hash, salt, calldata.span(), false) {
Result::Ok((addr, _)) => addr,
// TODO: do we want an event emitted here?
Result::Err(revert_reason) => panic(revert_reason)
}

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

Impact

  • Loss of NFT in case of unresponsive starknet.

Tools Used

Recommendations

- starknet::send_message_to_l1_syscall(
- self.bridge_l1_address.read().into(),
- buf.span(),
- )
- .unwrap_syscall();
+ match starknet::send_message_to_l1_syscall(
+ self.bridge_l1_address.read().into(), buf.span(),
+ ) {
+ Result::Ok(_) => {
+ self
+ .emit(
+ DepositRequestInitiated {
+ hash: req.hash,
+ block_timestamp: starknet::info::get_block_timestamp(),
+ req_content: req
+ }
+ );
+ },
+ Result::Err(revert_reason) => panic(revert_reason)
+ }
- self.emit(DepositRequestInitiated {
- hash: req.hash,
- block_timestamp: starknet::info::get_block_timestamp(),
- req_content: req
- });
Updates

Lead Judging Commences

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

Support

FAQs

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