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

When `bridge::deposit_tokens()` is called with `use_withdraw_auto: true` on L2, tokens will be lost because `bridge::withdrawTokens()` on L1 will always revert

0xnbvc

Vulnerability Details

Users can deposit tokens on L2 to be bridged on L1 by calling bridge::deposit_tokens():

bridge.cairo#L242-L250

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

The use_withdraw_auto bool parameter is used to automatically withdraw tokens on L1 using Starklane indexer, but has been disabled on L1 due following the last audit:

Bridge.sol#L153-L175

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);
}

However, the option is still available to users on the L2 bridge, therefore they can call bridge::deposit_tokens() with use_withdraw_auto: true and tokens will be stuck and lost because bridge::withdrawTokens() on L1 will always revert.

Impact

Loss of funds.

Recommendations

Hardcode use_withdraw_auto: false in bridge::deposit_tokens() on L2:

let req = Request {
- header: compute_request_header_v1(ctype, use_deposit_burn_auto, use_withdraw_auto),
+ header: compute_request_header_v1(ctype, use_deposit_burn_auto, false),
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(),
};
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year 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.

Give us feedback!