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.
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.
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.
Manual review
Get the msgHash and nonce when calling sendMessageToL2 from depositTokens function and return the nonce and payload to the caller of this function.
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.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.