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

Starknet tokens deposited with use_withdraw_auto can never be withdrawn

izcoser

Summary

The Starknet side of the bridge allows deposits with the option "use_withdraw_auto". If this argument is true, the user will never be able to withdraw on mainnet due to a revert after checking the request header.

Vulnerability Details

According to code commentary, automatic withdrawal on the mainnet side was disabled after an issue was found in the last audit.

However, this option is still available on the Starknet deposit side. If a user inadvertently passes use_withdraw_auto == true, his request header will always contain that unsupported option and will revert on Starklane::withdrawTokens at Bridge.sol:153.

Below is the problematic piece of code. The request header is checked and if it detects the auto withdrawal option, the withdrawal simply reverts while keeping the user's NFT, which can neither be withdrawn on L1 or unlocked back on Starknet.

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

POC

The POC is exactly what they included in the tests:

function test_withdrawTokensERC721AutoWithdrawDeploy() public {
// Build the request and compute it's "would be" message hash.
felt252 header = Protocol.requestHeaderV1(CollectionType.ERC721, false, true);
Request memory req = buildRequestDeploy(header, 888, bob);
uint256[] memory reqSerialized = Protocol.requestSerialize(req);
bytes32 msgHash = computeMessageHashFromL2(reqSerialized);
​
// The message must be simulated to come from the L2 indexer and registered
// as QUICK message.
​
IStarklane(bridge).addMessageHashForAutoWithdraw(uint256(msgHash));
vm.expectRevert(NotSupportedYetError.selector);
IStarklane(bridge).withdrawTokens(reqSerialized);
​
// okay, not supported yet...
// and what about the token I already deposited? >:(
}

Impact

Starknet native tokens will be LOCKED forever as they can't be withdrawn on mainnet.

Tools Used

Foundry.

Recommendations

On the Starknet side, revert the deposit if the user attempts to pass use_withdraw_auto == true.

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.

Appeal created

izcoser Submitter
10 months ago
n0kto Lead Judge
10 months ago
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.