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

Deposites from L2 to L1 will be unwithdrawable from L1 when activating `use_withdraw_auto`

alqaqa

Vulnerability Details

There was an issue related to Auto Withdrawals from L1 reported by Cairo_Security_Clan, and the team chose to remove this feature from the L1 side when withdrawing.

Bridge.sol#L169-L173

if (Protocol.canUseWithdrawAuto(header)) {
// 2024-03-19: disabled autoWithdraw after audit report
// _consumeMessageAutoWithdraw(_starklaneL2Address, request);
❌️ revert NotSupportedYetError();
} else { ... }

The problem is that this value is still taken as a parameter from the user in L2 Bridge when depositing Tokens. And no check guarantees that the value will be false, it is left to the user to set it.

bridge.cairo#L249

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,
) { ... }

So people Bridging From L2 still think that auto withdrawal is supported. But it is actually not. which will end up Locking for their NFT in the L2 Bridge and the inability to withdraw them from L1 as calling L1Bridge::withdrawTokens() will always revert.

Proof of Concept

  • UserA Wanted to Bridge one NFT from L2 to L1.

  • UserA thinks use_withdraw_auto is allowed as it is to him to either set or not set it.

  • UserA called L2Bridge::deposit_tokens() by setting use_withdraw_auto to true.

  • UserA Waited till Starknet verified the tx and put his message as an approved L2toL1 message.

  • UserA tried to withdraw his NFTs on L1 by calling L1Bridge::withdrawTokens().

  • Transactions get reverted whenever he tries to withdraw his token on L1.

Impact

Permanent Lock of NFTs on L2 Bridge Side and the inability to withdraw them from L1 side.

Tools Used

Manual Review

Recommended Mitigation

Ensure that use_withdraw_auto is set to false when withdrawing from L2 side.

diff --git a/apps/blockchain/starknet/src/bridge.cairo b/apps/blockchain/starknet/src/bridge.cairo
index 23cbf8a..5b82f1d 100644
--- a/apps/blockchain/starknet/src/bridge.cairo
+++ b/apps/blockchain/starknet/src/bridge.cairo
@@ -250,6 +250,7 @@ mod bridge {
) {
ensure_is_enabled(@self);
assert(!self.bridge_l1_address.read().is_zero(), 'Bridge is not open');
+ assert(use_withdraw_auto != true, "Auto Withdraw is Disappled");
// TODO: we may have the "from" into the params, to allow an operator
// to deposit token for a user.```
Updates

Lead Judging Commences

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

finding-auto_withdrawn-L2-NFT-stuck

Impact: High, token will be stuck in L2 bridge. Likelyhood: Very low, option is available in L2 but has been disabled since March on L1, would be almost a user error.

Support

FAQs

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