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

depositTokens function in Bridge.sol does not return nonce or payload from the function

Summary

depositTokens function in Bridge.sol does not return nonce or payload from the function. This means if the sender is an onchain smart contract then it will have no way to cancel the transaction if the original message was not consumed on Starknet side for any reason.

Vulnerability Details

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/IStarklane.sol#L21 - The definition of the function shows it returns no value to the caller.

sendMessageToL2 in Starknet Messaging contract returns a msgHash and nonce as shown here
https://github.com/starkware-libs/cairo-lang/blob/efa9648f57568aad8f8a13fbf027d2de7c63c2c0/src/starkware/starknet/solidity/StarknetMessaging.sol#L124

The nonce is required to cancel the transaction as shown here https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L225
and this is because the Starknet Messaging contract requires nonce to start message cancellation https://github.com/starkware-libs/cairo-lang/blob/efa9648f57568aad8f8a13fbf027d2de7c63c2c0/src/starkware/starknet/solidity/StarknetMessaging.sol#L151

The nonce is emitted by the sendMessageToL2 function in Starknet core messaging and thus offchain components can listen for this event and record the nonce. However, if the owner of the NFT is a smart contract then it will not know the nonce and will not be able to cancel the transaction.

The depositor also does not know how to serialize the request and form the payload which is required for message cancellation. Even this payload should be returned by the function. The payload is also emitted by depositTokens function and hence is not a problem for offchain indexers listening for the relevant event.

Impact

The impact could be very high and render the whole cancellation infrastructure unusable for onchain bots or contracts making the transfers on behalf of human users unless they develop their own infrastructure or contact the admin to retrieve the payload formed and nonce emitted by Starknet Messaging contract when sending the message to L2.

Tools Used

Manual review

Recommendations

Get the msgHash and nonce when calling sendMessageToL2 from depositTokens function and return the nonce and payload to the caller of this function.

(bytes32 msgHash , uint256 nonce) = IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);
return (payload, nonce); // Alternatively can also return msgHash which is useful for checking consumption status
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.

Support

FAQs

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