NFTBridge
60,000 USDC
View results
Submission Details
Severity: high
Invalid

The fixed `Payload` Length logic doesn't really enforce tokens don't get stuck in the bridge

Summary

The MAX_PAYLOAD_LENGTH constant in the Solidity contract is set to 300, which limits the size of the payload that can be sent in a depositTokens transaction. This has also been hinted in the walkthrough video to be done so as to ensure tokens are not locked in the bridge when too many are transferred. However, the actual transaction fee on StarkNet fluctuates based on various factors, including computational complexity, data availability, and the type of operations performed, even if the payload length remains constant. This discrepancy can lead to situations where transactions with payloads under the maximum length still exceed the gas limit, potentially causing tokens to be stuck on the bridge.

Vulnerability Details

When depositing this is called: https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L77-L144

function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
)
external
payable
{
if (!Cairo.isFelt252(snaddress.unwrap(ownerL2))) {
revert CairoWrapError();
}
if (!_enabled) {
revert BridgeNotEnabledError();
}
..snip
//@audit
if (payload.length >= MAX_PAYLOAD_LENGTH) {
revert TooManyTokensError();
}
IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);
emit DepositRequestInitiated(req.hash, block.timestamp, payload);
}

Now the MAX_PAYLOAD_LENGTH is hardcoded as 300, aiming to prevent transactions that exceed StarkNet's gas limit. However, the gas cost for transactions on StarkNet varies due to factors beyond payload size, such as computational steps, storage updates, and L2→L1 messages. This rigid limit may not account for the dynamic nature of gas costs, leading to transactions being rejected due to excessive fees even if they are under the payload limit.

More about how these gas costs can fluctuate can be read on here: https://github.com/starknet-io/starknet-docs/blob/main/components/Starknet/modules/architecture-and-concepts/pages/network-architecture/fee-mechanism.adoc#gas-and-transaction-fees.

Impact

Tokens could be stuck on the bridge if the transaction fee exceeds StarkNet's limit, despite the payload being within the allowed size. Users might face failed transactions and loss/stuck of funds due to the inability to predict exact gas costs, affecting the bridge's reliability and user experience.

Tools Used

Manual review

Recommendations

Instead of only using a fixed MAX_PAYLOAD_LENGTH, implement a dynamic estimation of transaction fees based on current network conditions and operation complexity. This approach would better align with StarkNet's fluctuating gas costs. Also cosider providing users with estimated fees before transaction submission, allowing them to adjust payload size or wait for optimal conditions.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Appeal created

bauchibred Submitter
9 months ago
n0kto Lead Judge
9 months ago
n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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