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

There is no guard against how much user will pay for fee

takarez

Summary

Vulnerability Details

the depositTokens function of the Bridge.sol allow users to send a token to the starknet:

/**
@notice Deposits token in escrow and initiates the
transfer to Starknet. Will revert if any of the token is missing approval
for the bridge as operator.
​
@param salt A salt used to generate the request hash.
@param collectionL1 Address of the collection contract.
@param ownerL2 New owner address on Starknet.
@param ids Ids of the token to transfer. At least 1 token is required.
*/

during the time of depositing and sending the message to the L2 contract, an amount of fee is required as per the starknet docs

The sequencer takes this fee in exchange for handling the message. The sequencer charges the fee in full upon updating the L1 state with the consumption of this message.

the issue with the current implementation is that, the fee is not handled appropriately as per the starknet , the ark protocol uses the msg.value as the amount of fee:

IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);
​

the problem here is that, there's no minimum or max amount a user can provide, meaning, if the user provides a huge amount, it'll be consumed by the sequencer without returning the excess amount

Impact

User providing a huge amount will be consumed and no refund will be made

Tools Used

manual review

Recommendations

Use the method provided by the starknet to quote the fee and get the estimate amount of it and also make sure user did not provide more than that

The fee itself is calculated in the same manner as "regular" L2 transactions. You can use the CLI to get an estimate of an L1 → L2 message fee.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Appeal created

takarez Submitter
9 months ago
n0kto Lead Judge
9 months ago
n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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