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

Users lose funds if `msg.value` for the deposit is less than 20k wei

x18a6

Summary

Users lose funds if msg.value for the deposit is less than 20k wei.

Vulnerability Details

Quoting L1-L2 messaging section from Cairo book:

It's important to note that we have {value: msg.value}. In fact, the minimum value we've to send here is 20k wei, due to the fact that the StarknetMessaging contract will register the hash of our message in the storage of Ethereum.
The problem is in Bridge::depositTokens(...), we don't check if the forwarded value is at least 20k wei, i.e. if a user forwards less than that it will be stuck in the bridge forever:

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L137-L141

function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
) external payable {
...
@>> IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address), felt252.unwrap(_starklaneL2Selector), payload
);
​
emit DepositRequestInitiated(req.hash, block.timestamp, payload);
}

Impact

  • Deposits that forwards less than 20k wei will revert because there's not enough gas to register the hash of the message in storage

  • The value forwarded is stuck in the bridge and lost.

Tools Used

Manual review.

Recommendations

function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
) external payable {
...
+ if(msg.value < 20_000) {
+ revert();
+ }
@>> IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address), felt252.unwrap(_starklaneL2Selector), payload
);
​
emit DepositRequestInitiated(req.hash, block.timestamp, payload);
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-not-enough-fee-can-block-NFT

Impact: Medium/High. Need an admin to start a cancellation and wait for 5 days once done. DoS > 5 days. Likelyhood: Low. Everytime a wallet/or a user do not send enough gas

Support

FAQs

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