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

Token cannot be withdrawn when users bridge ERC721 from starknet to Ethereum with use_withdraw_auto = true.

Summary

When users bridge tokens from starknet to Ethereum with use_withdraw_auto = true, the bridge process will be reverted, because use_withdraw_auto mode does not support in Ethereum.

Vulnerability Details

In bridge.cario, when we bridge NFT to Ethereum, we suppose to support different modes. We can set use_withdraw_auto is true or false.

In Ethereum bridge.sol, when we try to withdraw tokens bridged from Starknet, this behavior will be reverted. This will cause users' NFT is locked.
From the comments in bridge.sol, the auto withdrawal feature is disabled. However, in bridge.cario, we don't add any input parameter limitation.
As one normal user, the normal user goes through deposit_tokens all parameters, we cannot easily think use_withdraw_auto = true is the user's mistake.

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,
) {
...
let req = Request {
// ctype is one fixed value: ERC721
// @audit what if user set use_withdraw_auto = true
header: compute_request_header_v1(ctype, use_deposit_burn_auto, use_withdraw_auto),
...
}
function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
if (!_enabled) {
revert BridgeNotEnabledError();
}
// Header is always the first uint256 of the serialized request.
uint256 header = request[0];
// Any error or permission fail in the message consumption will cause a revert.
// After message being consumed, it is considered legit and tokens can be withdrawn.
if (Protocol.canUseWithdrawAuto(header)) {
// 2024-03-19: disabled autoWithdraw after audit report
// _consumeMessageAutoWithdraw(_starklaneL2Address, request);
@> revert NotSupportedYetError();
}
...
}

Impact

Users' NFT will be locked.

Tools Used

Manual

Recommendations

Add input parameter check for deposit_token in cairo bridge.

Updates

Lead Judging Commences

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