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

Starknet users bridging his NFTs with the `use_withdraw_auto` set to true will block his NFTs forever

draiakoo

Summary

Starknet users bridging his NFTs with the use_withdraw_auto set to true will block his NFTs forever

Vulnerability Details

When a user wants to bridge an NFT from Starknet to Ethereum, he has to call the following function:

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

In this function we can notice that there is a particular parameter use_withdraw_auto that is set by the user and inside the function has no specific check nor constrain. Thus, it can be set either true or false by the user. The problem arises when the message gets delivered to Ethereum and the user wants to withdraw the NFT.

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
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);
}
​
...
}

As we can see, when the user will pass the request que made, it will extract this parameter from the header and the code will enter the first if branch which essentially will make the transaction to revert because the functionality has been disabled.

The final result will be the user losing his NFT in Starknet because he will not be able to withdraw it from Ethereum.

Impact

Medium, the severity is high, the user will lose his NFTs. However, it is a user mistake so a medium severity would be good.

Tools Used

Manual review

Recommendations

While the auto withdraw functionality remains blocked, add a check in the Starknet bridge to ensure this parameter is false:

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,
) {
+ // Auto withdraw feature is currently blocked
+ assert(use_withdraw_auto == false, "Auto withdraw disabled");
​
...
​
}
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!