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

Bridging from L2 to L1 may revert if users provide the `use_withdraw_auto` with `true`, causing NFTs to get stuck on L1

said

Summary

When users bridge NFTs from L2 to L1, they can provide the use_withdraw_auto flag. However, if use_withdraw_auto is set to true, their withdrawTokens calls on L1 will always revert, causing the NFTs to get stuck on L1.

Vulnerability Details

When users bridge nft from L2 to L1, they allowed to provide use_withdraw_auto flag, this flag will be encoded inside the header and propagated to L1 alongside with other Request data.

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

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,
) {
ensure_is_enabled(@self);
assert(!self.bridge_l1_address.read().is_zero(), 'Bridge is not open');
​
// ...
​
let collection_l1 = self.l2_to_l1_addresses.read(collection_l2);
​
let req = Request {
>>> header: compute_request_header_v1(ctype, use_deposit_burn_auto, use_withdraw_auto),
hash: compute_request_hash(salt, collection_l2, owner_l1, token_ids),
collection_l1,
collection_l2,
owner_l1,
owner_l2: from,
name,
symbol,
base_uri,
ids: token_ids,
values: array![].span(),
uris,
new_owners: array![].span(),
};
// ....
}

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

fn compute_request_header_v1(
ctype: CollectionType,
use_deposit_burn_auto: bool,
use_withdraw_auto: bool,
) -> felt252 {
let mut header: u256 = HEADER_V1;
​
if (ctype == CollectionType::ERC721) {
header = header | ERC721_TYPE;
} else if (ctype == CollectionType::ERC1155) {
header = header | ERC1155_TYPE;
} else {
panic_with_felt252('Invalid collection type');
}
​
if (use_deposit_burn_auto) {
header = header | DEPOSIT_AUTO_BURN;
}
// const WITHDRAW_AUTO: u256 = 0x01000000;
>>> if (use_withdraw_auto) {
header = header | WITHDRAW_AUTO;
}
​
header.try_into().expect('Invalid header V1')
}

However, on L1, when users call withdrawTokens to retrieve the token, the transaction will revert because withdraw auto feature is currently disabled on L1.

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

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();
} else {
_consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request);
}
// ...
}

This will cause users call to retrieve the tokens will always revert.

Impact

Users can't retrieve their NFT on L1, and there is no way to recover the NFT on L2.

Tools Used

Manual review

Recommendations

If users set use_withdraw_auto to true, revert the bridging request on L2. Additionally, consider implementing a recover NFT functionality on L2 in case another unexpected issue arises.

Updates

Lead Judging Commences

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