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

Unverified Request Payload Processing in `withdrawTokens` Function

0xshoonya

Summary

withdrawTokens function of the L1 bridge contract processes data from the L2 message payload before verifying its authenticity through the StarkNet messaging system. This violates the security model outlined in the Cairo/Starknet documentation and could lead to processing of unverified, potentially malicious data.

Vulnerability Details

The withdrawTokens function in the L1 bridge contract is designed to handle withdrawal requests from the StarkNet L2 network.

The vulnerability stems from an incorrect order of operations in the withdrawTokens function. This function currently extracts the header from the request payload before verifying the message's authenticity through the _consumeMessageStarknet function. This order of operations poses a security risk because unverified data is used before cryptographic validation, violating the principles outlined in the StarkNet documentation.

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];
​
// ...
​
if (Protocol.canUseWithdrawAuto(header)) {
// 2024-03-19: disabled autoWithdraw after audit report
// _consumeMessageAutoWithdraw(_starklaneL2Address, request);
revert NotSupportedYetError();
} else {
@> _consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request);
}
​
// ...
​
CollectionType ctype = Protocol.collectionTypeFromHeader(header);
​
// ...
}

According to the StarkNet documentation, messages from L2->L1 must be consumed and verified before their contents are used. The correct sequence, as implied by the documentation, is:

  1. Consume and verify the message using consumeMessageFromL2.

  2. Extract and use the payload data.

This is the code example provided in the cairo-book::L1<>L2Messaging:

function consumeMessageFelt(
uint256 fromAddress,
uint256[] calldata payload
)
external
{
let messageHash = _snMessaging.consumeMessageFromL2(fromAddress, payload);
​
// You can use the message hash if you want here.
​
// We expect the payload to contain only a felt252 value (which is a uint256 in Solidity).
require(payload.length == 1, "Invalid payload");
​
uint256 my_felt = payload[0];
​
// From here, you can safely use `my_felt` as the message has been verified by StarknetMessaging.
require(my_felt > 0, "Invalid value");
}

You can see that the payload is used here only after the message has been verified by StarknetMessaging i.e. after a call to consumeMessageFromL2 function.

Impact

By processing the header before message verification, the contract assumes the integrity of data before it has been cryptographically verified by the StarkNet system. This violates the StarkNet L2->L1 messaging security model.
This could potentially open up unforeseen attack vectors for e.g. it could enable attacker to manipulate withdrawal logic, leading to unauthorized token withdrawals or the deployment of malicious contracts on L1

Tools Used

Manual Review, Cairo Book

Recommendations

Request payload must only be processed after message verification:

function withdrawTokens(uint256[] calldata request) external payable returns (address) {
if (!_enabled) {
revert BridgeNotEnabledError();
}
​
- uint256 header = request[0];
​
if (Protocol.canUseWithdrawAuto(header)) {
// 2024-03-19: disabled autoWithdraw after audit report
// _consumeMessageAutoWithdraw(_starklaneL2Address, request);
revert NotSupportedYetError();
} else {
_consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request);
}
​
+ uint256 header = request[0];
​
// ...
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!